Repository: CERT-Polska/mwdb-core (language: python)

Issue:
Refactor of the user login view
**Feature Category**

- [ ] Correctness
- [x] User Interface / User Experience
- [ ] Performance
- [ ] Other (please explain)

**Describe the problem**

<!-- A clear and concise description of the problem. -->
The current user login view is a bit inconsistent.
The page in a separate place contains login button, in yet another redirection to registration, 
while logging in using Oauth seems to be strongly separated from the rest and can be omitted by the user.

**Describe the solution you'd like**

<!-- If you have a solution in mind, please describe it. -->
Design a new view that meets current standards and clearly includes all functionalities.

Example below:
![image](https://user-images.githubusercontent.com/63305564/209808709-5c08231c-1f6d-4b83-90fb-33fd44024247.png)





Discussion / hints:

Fix patch:
diff --git a/mwdb/web/src/App.js b/mwdb/web/src/App.js
index f66a37ef6..b065a0bfd 100644
--- a/mwdb/web/src/App.js
+++ b/mwdb/web/src/App.js
@@ -38,7 +38,7 @@ import ProfileAPIKeys from "./components/Profile/Views/ProfileAPIKeys";
 import ProfileResetPassword from "./components/Profile/Views/ProfileResetPassword";
 import ProfileOAuth from "./components/Profile/Views/ProfileOAuth";
 
-import { OAuthLogin, OAuthAuthorize } from "./components/OAuth";
+import { OAuthAuthorize } from "./components/OAuth";
 
 import SettingsOverview from "./components/Settings/Views/SettingsOverview";
 import UsersPendingList from "./components/Settings/Views/UsersPendingList";
@@ -100,20 +100,12 @@ function SampleRouteFallback() {
 }
 
 function AppRoutes() {
-    const {
-        config: { is_registration_enabled: isRegistrationEnabled },
-    } = useContext(ConfigContext);
     return (
         <Routes>
             <Route path="login" element={<UserLogin />} />
-            {isRegistrationEnabled ? (
-                <Route path="register" element={<UserRegister />} />
-            ) : (
-                []
-            )}
+            <Route path="register" element={<UserRegister />} />
             <Route path="recover_password" element={<UserPasswordRecover />} />
             <Route path="setpasswd/:token" element={<UserSetPassword />} />
-            <Route path="oauth/login" element={<OAuthLogin />} />
             <Route path="oauth/callback" element={<OAuthAuthorize />} />
             <Route element={<RequiresAuth />}>
                 <Route path="/" element={<RecentSamples />} />
diff --git a/mwdb/web/src/components/Navigation.js b/mwdb/web/src/components/Navigation.js
index fcf52d034..d35207d86 100644
--- a/mwdb/web/src/components/Navigation.js
+++ b/mwdb/web/src/components/Navigation.js
@@ -91,25 +91,6 @@ export default function Navigation() {
     const remotePath = useRemotePath();
     const navItems = config.isReady ? (
         <Extendable ident="navbar">
-            {!auth.isAuthenticated &&
-            config.config["is_registration_enabled"] ? (
-                <li className="nav-item">
-                    <Link className="nav-link" to={"/register"}>
-                        Register user
-                    </Link>
-                </li>
-            ) : (
-                []
-            )}
-            {!auth.isAuthenticated && config.config["is_oidc_enabled"] ? (
-                <li className="nav-item">
-                    <Link className="nav-link" to={"/oauth/login"}>
-                        OAuth authentication
-                    </Link>
-                </li>
-            ) : (
-                []
-            )}
             {auth.isAuthenticated ? (
                 <Extendable ident="navbarAuthenticated">
                     <li className="nav-item">
diff --git a/mwdb/web/src/components/OAuth.js b/mwdb/web/src/components/OAuth.js
index 7ceee3e7c..d5638cdcb 100644
--- a/mwdb/web/src/components/OAuth.js
+++ b/mwdb/web/src/components/OAuth.js
@@ -1,144 +1,87 @@
 import React, { useContext, useEffect, useState } from "react";
-import { Link, useSearchParams, useNavigate } from "react-router-dom";
+import { useSearchParams, useNavigate } from "react-router-dom";
 
-import { APIContext } from "@mwdb-web/commons/api/context";
+import api from "@mwdb-web/commons/api";
 import { AuthContext } from "@mwdb-web/commons/auth";
-import { View, getErrorMessage } from "@mwdb-web/commons/ui";
-import { ConfirmationModal, ShowIf } from "@mwdb-web/commons/ui";
+import { getErrorMessage } from "@mwdb-web/commons/ui";
 
-export function OAuthLogin() {
-    const api = useContext(APIContext);
-    const [error, setError] = useState();
-    const [providers, setProviders] = useState([]);
-    const [chosenProvider, setChosenProvider] = useState();
-    const [isRedirectModalOpen, setRedirectModalOpen] = useState(false);
-    const [isRegisterModalOpen, setRegisterModalOpen] = useState(false);
-
-    async function getProviders() {
-        try {
-            const response = await api.oauthGetProviders();
-            setProviders(response.data["providers"]);
-        } catch (e) {
-            setError(e);
-        }
+async function authenticate(provider, action, errorFunction) {
+    try {
+        const response = await api.oauthAuthenticate(provider);
+        const expirationTime = Date.now() + 5 * 60 * 1000;
+        sessionStorage.setItem(
+            `openid_${response.data["state"]}`,
+            JSON.stringify({
+                provider: provider,
+                nonce: response.data["nonce"],
+                action: action,
+                expiration: expirationTime,
+            })
+        );
+        window.location = response.data["authorization_url"];
+    } catch (e) {
+        errorFunction(e);
     }
+}
 
-    async function authenticate(provider, action) {
-        try {
-            const response = await api.oauthAuthenticate(provider);
-            const expirationTime = Date.now() + 5 * 60 * 1000;
-            sessionStorage.setItem(
-                `openid_${response.data["state"]}`,
-                JSON.stringify({
-                    provider: provider,
-                    nonce: response.data["nonce"],
-                    action: action,
-                    expiration: expirationTime,
-                })
-            );
-            window.location = response.data["authorization_url"];
-        } catch (e) {
-            setError(e);
-        }
-    }
+export function ProviderButton({ provider, color }) {
+    // todo: correct error handling
+    const [, setError] = useState();
+    const chosenProvider = provider;
 
-    useEffect(() => {
-        getProviders();
-        // eslint-disable-next-line react-hooks/exhaustive-deps
-    }, []);
+    return (
+        <button
+            onClick={(e) => {
+                e.preventDefault();
+                authenticate(chosenProvider, "authorize", setError);
+            }}
+            className="form-control btn btn-primary mb-1"
+            style={{
+                backgroundColor: color,
+                borderStyle: "none",
+            }}
+        >
+            Log in with {provider}
+        </button>
+    );
+}
+
+export function ProvidersSelectList({ providersList }) {
+    // todo: correct error handling
+    const [, setError] = useState();
+    const availableProviders = providersList;
+    const [chosenProvider, setChosenProvider] = useState();
 
     return (
-        <View error={error}>
-            <h2>External authentication</h2>
-            <p>
-                Select below the identity provider associated with your mwdb
-                account. By clicking on the identity provider below you will be
-                redirected to its authentication page. <br />
-                If you don't have an account associated with any of these
-                providers you can do this in the profile details. <br />
-                Alternatively, you can register a new account through an
-                external identity provider by{" "}
-                <Link
-                    to="#"
-                    onClick={(ev) => {
-                        ev.preventDefault();
-                        setRegisterModalOpen(true);
-                    }}
-                >
-                    clicking here
-                </Link>
-                .
-            </p>
-            <ShowIf condition={providers.length}>
-                {providers.map((provider) => (
-                    <div className="d-flex justify-content-center">
-                        <div className="col-6 text-center">
-                            <Link
-                                to="#"
-                                className="card btn-outline-secondary text-decoration-none"
-                                onClick={(ev) => {
-                                    ev.preventDefault();
-                                    setChosenProvider(provider);
-                                    setRedirectModalOpen(true);
-                                }}
-                            >
-                                <div className="card-body">
-                                    <h5>{provider}</h5>
-                                </div>
-                            </Link>
-                        </div>
-                    </div>
-                ))}
-            </ShowIf>
-            <ConfirmationModal
-                isOpen={isRedirectModalOpen}
-                onRequestClose={() => {
-                    setRedirectModalOpen(false);
-                    setChosenProvider("");
-                }}
-                onConfirm={(e) => {
-                    e.preventDefault();
-                    authenticate(chosenProvider, "authorize");
+        <form>
+            <select
+                className="custom-select"
+                onChange={(e) => {
+                    setChosenProvider(e.target.value);
                 }}
-                message={`Are you sure you want to redirect to ${chosenProvider} provider`}
-                buttonStyle="btn-danger"
-            />
-            <ConfirmationModal
-                isOpen={isRegisterModalOpen}
-                onRequestClose={() => setRegisterModalOpen(false)}
-                message="Choose OpenID Provider to register user"
-                onConfirm={(e) => {
+            >
+                <option value="" hidden>
+                    Select provider...
+                </option>
+                {availableProviders.map((provider) => (
+                    <option value={provider}>{provider}</option>
+                ))}
+            </select>
+            <button
+                className="form-control btn btn-primary mt-1"
+                style={{ backgroundColor: "#3c5799" }}
+                onClick={(e) => {
                     e.preventDefault();
-                    authenticate(chosenProvider, "register");
+                    authenticate(chosenProvider, "authorize", setError);
                 }}
-                buttonStyle="btn-info"
-                confirmText="Submit"
             >
-                <form onSubmit={(e) => {}}>
-                    <div>
-                        <select
-                            className="custom-select"
-                            value={chosenProvider}
-                            onChange={(ev) =>
-                                setChosenProvider(ev.target.value)
-                            }
-                        >
-                            <option value="" hidden>
-                                Select provider...
-                            </option>
-                            {providers.map((provider) => (
-                                <option value={provider}>{provider}</option>
-                            ))}
-                        </select>
-                    </div>
-                </form>
-            </ConfirmationModal>
-        </View>
+                Log in with selected provider
+            </button>
+        </form>
     );
 }
 
 export function OAuthAuthorize() {
-    const api = useContext(APIContext);
     const auth = useContext(AuthContext);
     const navigate = useNavigate();
     // Current query set in URI path
@@ -189,7 +132,7 @@ export function OAuthAuthorize() {
                     replace: true,
                 });
             else {
-                navigate("/oauth/login", {
+                navigate("/login", {
                     state: {
                         error: getErrorMessage(e),
                     },
diff --git a/mwdb/web/src/components/UserLogin.js b/mwdb/web/src/components/UserLogin.js
index 0c0f8d621..351c45674 100644
--- a/mwdb/web/src/components/UserLogin.js
+++ b/mwdb/web/src/components/UserLogin.js
@@ -1,19 +1,26 @@
-import React, { useContext, useState } from "react";
+import React, { useCallback, useContext, useEffect, useState } from "react";
 import { Link, Navigate, useNavigate, useLocation } from "react-router-dom";
 
 import { AuthContext } from "@mwdb-web/commons/auth";
+import { ConfigContext } from "@mwdb-web/commons/config";
 import api from "@mwdb-web/commons/api";
 import { Extension } from "@mwdb-web/commons/extensions";
-import { View } from "@mwdb-web/commons/ui";
+import { View, ShowIf } from "@mwdb-web/commons/ui";
+import { ProviderButton, ProvidersSelectList } from "./OAuth";
 
 export default function UserLogin() {
     const auth = useContext(AuthContext);
+    const config = useContext(ConfigContext);
     const navigate = useNavigate();
     const location = useLocation();
 
     const [login, setLogin] = useState("");
     const [password, setPassword] = useState("");
     const [loginError, setLoginError] = useState(null);
+    const [providers, setProviders] = useState([]);
+
+    const colorsList = ["#3c5799", "#01a0f6", "#d03f30", "#b4878b", "#444444"];
+    const isOIDCEnabled = config.config["is_oidc_enabled"];
 
     const locationState = location.state || {};
     async function tryLogin() {
@@ -27,49 +34,100 @@ export default function UserLogin() {
         }
     }
 
+    const getProviders = useCallback(async () => {
+        try {
+            const response = await api.oauthGetProviders();
+            setProviders(response.data["providers"]);
+        } catch (e) {
+            setLoginError(e);
+        }
+    }, []);
+
+    useEffect(() => {
+        if (isOIDCEnabled) {
+            getProviders();
+        }
+    }, [getProviders, isOIDCEnabled]);
+
     if (auth.isAuthenticated) return <Navigate to="/" />;
 
     return (
-        <View ident="userLogin" error={loginError}>
-            <h2>Login</h2>
-            <form
-                onSubmit={(ev) => {
-                    ev.preventDefault();
-                    tryLogin();
-                }}
-            >
-                <Extension ident="userLoginNote" />
-                <div className="form-group">
-                    <label>Login</label>
-                    <input
-                        type="text"
-                        name="login"
-                        value={login}
-                        onChange={(ev) => setLogin(ev.target.value)}
-                        className="form-control"
-                        required
-                    />
-                </div>
-                <div className="form-group">
-                    <label>Password</label>
+        <div className="user-login">
+            <div className="background" />
+            <View fluid ident="userLogin" error={loginError}>
+                <h2 align="center">Welcome to MWDB</h2>
+                <h6 align="center">Log in using mwdb credentials</h6>
+                <form
+                    onSubmit={(ev) => {
+                        ev.preventDefault();
+                        tryLogin();
+                    }}
+                >
+                    <Extension ident="userLoginNote" />
+                    <div className="form-group">
+                        <label>Login</label>
+                        <input
+                            type="text"
+                            name="login"
+                            value={login}
+                            onChange={(ev) => setLogin(ev.target.value)}
+                            className="form-control"
+                            required
+                        />
+                    </div>
+                    <div className="form-group">
+                        <label>Password</label>
+                        <input
+                            type="password"
+                            name="password"
+                            value={password}
+                            onChange={(ev) => setPassword(ev.target.value)}
+                            className="form-control"
+                            required
+                        />
+                    </div>
                     <input
-                        type="password"
-                        name="password"
-                        value={password}
-                        onChange={(ev) => setPassword(ev.target.value)}
-                        className="form-control"
-                        required
+                        type="submit"
+                        value="Log in"
+                        className="form-control btn btn-success"
                     />
-                </div>
-                <nav className="form-group">
-                    <Link to="/recover_password">Forgot password?</Link>
-                </nav>
-                <input
-                    type="submit"
-                    value="Submit"
-                    className="btn btn-primary"
-                />
-            </form>
-        </View>
+                    <nav
+                        className="form-group"
+                        style={{ textAlign: "center", marginTop: "5px" }}
+                    >
+                        <div className="d-flex justify-content-between">
+                            <div>
+                                <Link to="/recover_password">
+                                    Forgot password?
+                                </Link>
+                            </div>
+                            <div>
+                                <ShowIf
+                                    condition={
+                                        config.config["is_registration_enabled"]
+                                    }
+                                >
+                                    <Link to="/register">Register user</Link>
+                                </ShowIf>
+                            </div>
+                        </div>
+                    </nav>
+                    <ShowIf condition={providers.length}>
+                        <hr />
+                        <h6 align="center">Log in using OAuth</h6>
+                        {providers.length <= 5 ? (
+                            providers.map((provider, i) => (
+                                <ProviderButton
+                                    provider={provider}
+                                    color={colorsList[i % colorsList.length]}
+                                />
+                            ))
+                        ) : (
+                            <ProvidersSelectList providersList={providers} />
+                        )}
+                    </ShowIf>
+                </form>
+            </View>
+        </div>
     );
 }
diff --git a/mwdb/web/src/styles/index.css b/mwdb/web/src/styles/index.css
index 28332c8ab..c2c8b566a 100644
--- a/mwdb/web/src/styles/index.css
+++ b/mwdb/web/src/styles/index.css
@@ -168,6 +168,40 @@ pre.attribute-object {
     display: inline-block;
 }
 
+/***
+ User login CSS
+ ***/
+
+div.user-login .container-fluid {
+    background-color: white;
+    max-width: 450px;
+    border-radius: 10px;
+}
+
+@media (min-width: 576px) {
+    div.user-login .background {
+        background-color: #f2f2f2;
+        position: fixed;
+        top: 0;
+        left: 0;
+        z-index: -1;
+        height: 100%;
+        width: 100%;
+    }
+
+    div.user-login .container-fluid {
+        margin-top: calc(-12pt + 3rem);
+        padding: 3rem;
+    }
+}
+
+@media (max-width: 576px) {
+    div.user-login .container-fluid {
+        margin-top: -12pt;
+        padding: 2rem 2rem;
+    }
+}
+
 /***
 	Material Design Switches for Bootstrap 4 and Material Design Bootstrap (MDB)
 	by djibe


Test patch:
diff --git a/tests/frontend/cypress/integration/util.js b/tests/frontend/cypress/integration/util.js
index d22a770fc..51aa1ad4f 100644
--- a/tests/frontend/cypress/integration/util.js
+++ b/tests/frontend/cypress/integration/util.js
@@ -16,6 +16,6 @@ export const browserLogin = (username, password) => {
 
   cy.get("input[name=password]").type(password).should("have.value", password);
 
-  cy.contains("Submit").click();
+  cy.get('input').contains("Log in").click();
   cy.get("a.profile-button").should("have.attr", "href").and('contain', '/profile');
 };