Skip to content

Commit

Permalink
fix: use direct links for logging in (#3339)
Browse files Browse the repository at this point in the history
Fixes #3338, fixes SwissDataScienceCenter/renku-gateway#729.

Remove the `/login` page (it was only doing a redirect) and replace all links to `/login` with links to `/api/auth/login` which is the login endpoint.

Doing the redirect in-app added unnecessary complexity and caused HTTP 500 errors with the new gateway.
  • Loading branch information
leafty authored Oct 9, 2024
1 parent 555c855 commit ae18305
Show file tree
Hide file tree
Showing 34 changed files with 223 additions and 298 deletions.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,6 @@ flowchart LR
subgraph L1
A(/)-->DA(/datasets)
A-->HE(/help)
A-->LO(/login)
A-->LOOUT(/logout)
A-->PR(/projects)
A-->SEA(/search)
Expand Down
7 changes: 1 addition & 6 deletions client/src/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { Route, Switch } from "react-router-dom";
import { CompatRoute } from "react-router-dom-v5-compat";
import { ToastContainer } from "react-toastify";

import { LoginHelper, LoginRedirect } from "./authentication";
import { LoginHelper } from "./authentication";
import { Loader } from "./components/Loader";
import { DatasetCoordinator } from "./dataset/Dataset.state";
import LazyShowDataset from "./dataset/LazyShowDataset";
Expand Down Expand Up @@ -103,11 +103,6 @@ function CentralContentContainer(props) {
<title>Reproducible Data Science | Open Research | Renku</title>
</Helmet>
<Switch>
<CompatRoute exact path="/login">
<ContainerWrap fullSize>
<LoginRedirect />
</ContainerWrap>
</CompatRoute>
<CompatRoute exact path="/">
{props.user.logged ? (
<ContainerWrap>
Expand Down
6 changes: 5 additions & 1 deletion client/src/App.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ import { createCoreApiVersionedUrlConfig } from "./utils/helpers/url";

describe("rendering", () => {
const model = new StateModel(globalSchema);
const params = { WELCOME_PAGE: "Some text", STATUSPAGE_ID: "5bce9beff4ca" };
const params = {
WELCOME_PAGE: "Some text",
STATUSPAGE_ID: "5bce9beff4ca",
GATEWAY_URL: "https://renkulab.io/api",
};
const fakeLocation = { pathname: "" };

it("renders anonymous user without crashing", async () => {
Expand Down
20 changes: 7 additions & 13 deletions client/src/authentication/Authentication.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@
*/
import { describe, expect, it, vi } from "vitest";

import { LoginHelper } from "./Authentication.container";
import { createLoginUrl } from "./LoginRedirect";
import { LoginHelper, RenkuQueryParams } from "./Authentication.container";

// Mock relevant react objects
const location = { pathname: "", state: "", previous: "", search: "" };
Expand Down Expand Up @@ -62,24 +61,19 @@ async function dispatchStorageEvent(key, newValue) {
describe("LoginHelper functions", () => {
const { queryParams } = LoginHelper;

it("createLoginUrl", async () => {
const extraParam = `${queryParams.login}=${queryParams.loginValue}`;

expect(createLoginUrl(url)).toBe(`${url}?${extraParam}`);

const urlWithParam = `${url}?test=1`;
expect(createLoginUrl(urlWithParam)).toBe(`${urlWithParam}&${extraParam}`);
});

it("handleLoginParams", async () => {
localStorage.clear();

LoginHelper.handleLoginParams(history);
expect(localStorage.length).toBe(0);
const loginUrl = createLoginUrl(url);
const loginUrl = new URL(url);
loginUrl.searchParams.set(
RenkuQueryParams.login,
RenkuQueryParams.loginValue
);
const loginHistory = {
...history,
location: { ...location, search: loginUrl.replace(url, "") },
location: { ...location, search: loginUrl.href.replace(url, "") },
};
const datePre = new Date().getTime();

Expand Down
60 changes: 0 additions & 60 deletions client/src/authentication/LoginRedirect.test.jsx

This file was deleted.

59 changes: 0 additions & 59 deletions client/src/authentication/LoginRedirect.tsx

This file was deleted.

3 changes: 1 addition & 2 deletions client/src/authentication/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
* limitations under the License.
*/

import LoginRedirect from "./LoginRedirect.tsx";
import { LoginHelper } from "./Authentication.container.js";

export { LoginHelper, LoginRedirect };
export { LoginHelper };
66 changes: 66 additions & 0 deletions client/src/authentication/useLoginUrl.hook.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*!
* Copyright 2024 - Swiss Data Science Center (SDSC)
* A partnership between École Polytechnique Fédérale de Lausanne (EPFL) and
* Eidgenössische Technische Hochschule Zürich (ETHZ).
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { useContext, useEffect, useRef } from "react";
import { useLocation } from "react-router-dom-v5-compat";

import AppContext from "../utils/context/appContext";
import type { AppParams } from "../utils/context/appParams.types";
import { RenkuQueryParams } from "./Authentication.container";

interface UseLoginUrlArgs {
params?: AppParams;
redirectUrl?: URL;
}

export function useLoginUrl(args?: UseLoginUrlArgs): URL {
const { params: params_, redirectUrl: redirectUrl_ } = args ?? {};

const { params: appContextParams } = useContext(AppContext);
const params = params_ ?? appContextParams;
const gatewayUrl = params?.GATEWAY_URL;

const location = useLocation();
const windowLocationRef = useRef<string>(window.location.href);

useEffect(() => {
windowLocationRef.current = window.location.href;
}, [location]);

if (!gatewayUrl) {
throw new Error("Cannot create login URL");
}

const redirectUrl =
redirectUrl_ ?? windowLocationRef.current
? new URL(windowLocationRef.current)
: null;
if (redirectUrl && !redirectUrl.search.includes(RenkuQueryParams.login)) {
redirectUrl.searchParams.append(
RenkuQueryParams.login,
RenkuQueryParams.loginValue
);
}

const loginUrl = new URL(`${gatewayUrl}/auth/login`);
if (redirectUrl) {
loginUrl.searchParams.set("redirect_url", redirectUrl.href);
}

return loginUrl;
}
13 changes: 5 additions & 8 deletions client/src/components/loginAlert/LoginAlert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@
* LoginAlert component
*/

import cx from "classnames";
import { Alert } from "reactstrap";
import { Link, useLocation } from "react-router-dom";

import { Url } from "../../utils/helpers/url";
import { useLoginUrl } from "../../authentication/useLoginUrl.hook";

export interface LoginAlertProps {
logged: boolean;
Expand All @@ -45,18 +45,15 @@ const LoginAlert = ({
textPost = " to use this feature.",
textPre,
}: LoginAlertProps) => {
const location = useLocation();
const loginUrl = useLoginUrl();

// No need to show anything when the user is logged.
if (logged) return null;

const loginUrl = Url.get(Url.pages.login.link, {
pathname: location.pathname,
});
const link = (
<Link className="btn btn-primary btn-sm" to={loginUrl}>
<a className={cx("btn", "btn-primary", "btn-sm")} href={loginUrl.href}>
{textLogin}
</Link>
</a>
);
const introElement = textIntro ? <p>{textIntro}</p> : null;

Expand Down
3 changes: 1 addition & 2 deletions client/src/components/navbar/LoggedInNavBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ export default function LoggedInNavBar({
/>
</NavItem>
<NavItem className="nav-item col-auto">
{/* eslint-disable-next-line @typescript-eslint/no-explicit-any */}
<RenkuToolbarItemUser params={params as any} />
<RenkuToolbarItemUser params={params} />
</NavItem>
</Nav>
</Collapse>
Expand Down
15 changes: 9 additions & 6 deletions client/src/components/navbar/NavBarItems.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@ import {
DropdownItem,
DropdownMenu,
DropdownToggle,
NavLink,
UncontrolledDropdown,
} from "reactstrap";

import { LoginHelper } from "../../authentication";
import { useLoginUrl } from "../../authentication/useLoginUrl.hook";
import AdminDropdownItem from "../../landing/AdminDropdownItem";
import { User } from "../../model/renkuModels.types";
import NotificationsMenu from "../../notifications/NotificationsMenu";
Expand All @@ -43,10 +45,8 @@ import {
getActiveProjectPathWithNamespace,
gitLabUrlFromProfileUrl,
} from "../../utils/helpers/HelperFunctions";
import { Url } from "../../utils/helpers/url";
import { ExternalDocsLink, ExternalLink } from "../ExternalLinks";
import { Loader } from "../Loader";
import { RenkuNavLink } from "../RenkuNavLink";
import BootstrapGitLabIcon from "../icons/BootstrapGitLabIcon";

import styles from "./NavBarItem.module.scss";
Expand Down Expand Up @@ -284,8 +284,6 @@ interface RenkuToolbarItemUserProps {
}

export function RenkuToolbarItemUser({ params }: RenkuToolbarItemUserProps) {
const location = useLocation();

const user = useLegacySelector<User>((state) => state.stateModel.user);

const { renku10Enabled } = useAppSelector(({ featureFlags }) => featureFlags);
Expand All @@ -294,11 +292,16 @@ export function RenkuToolbarItemUser({ params }: RenkuToolbarItemUserProps) {
const uiserverURL = params.UISERVER_URL;
const redirect_url = encodeURIComponent(params.BASE_URL);

const loginUrl = useLoginUrl({ params });

if (!user.fetched) {
return <Loader inline size={16} />;
} else if (!user.logged) {
const to = Url.get(Url.pages.login.link, { pathname: location.pathname });
return <RenkuNavLink className="px-2" to={to} title="Login" />;
return (
<NavLink className="px-2" href={loginUrl.href}>
Login
</NavLink>
);
}

return (
Expand Down
Loading

0 comments on commit ae18305

Please sign in to comment.