Skip to content

Split Login and Register, enable OIDC Registration. #20287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 20 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
5bf8c81
Add Config opion disable_local_accounts
uwwint May 9, 2025
34718a7
Rename allow_user_creation to allow_local_account_creation
uwwint May 9, 2025
bfff3a9
add deprecation message for allow_user_creation on galaxy startup
uwwint May 9, 2025
3d1b917
add logic to disable local account creation when local accounts are d…
uwwint May 9, 2025
828aa4e
Split Login and Registration. Add a new route /register/start duplica…
uwwint May 9, 2025
ec91604
If local account creation is disabled do not show username and passwo…
uwwint May 9, 2025
4487c19
rebuild-config
uwwint May 13, 2025
3d56ad6
add configuration option "end_user_registration_endpoint" to OIDC pro…
uwwint May 13, 2025
579fda6
refactor ExternalLogin into helper and template implementation. Needs…
uwwint May 13, 2025
915b3b5
Login will now redirect in case there is only one OIDC provider and l…
uwwint May 13, 2025
c5fa08c
If local accounts are disabled and there is only one OIDC registratio…
uwwint May 13, 2025
d255066
Fix: if local accounts are enabled, the login page should be shown wh…
uwwint May 13, 2025
695df7d
Fix: Registration page should be shown if local accounts are configured
uwwint May 13, 2025
fd576c2
Fix: incorrect reference to config.disable_local_accouns leading to n…
uwwint May 14, 2025
aaacbfa
Seperate implementation of registration and login, fix all test cases…
uwwint May 19, 2025
ffb8792
add test cases for registration
uwwint May 19, 2025
dc34986
linting fixes, other clean up, fixes for various conditionals
ahmedhamidawan May 22, 2025
49f6084
adjust `ExternalRegistration` jest for changes
ahmedhamidawan May 22, 2025
15b813b
fix ruff errors
ahmedhamidawan May 22, 2025
c2f3606
use `GLink` and `GButton` in login/registration; replacing `openurl` …
ahmedhamidawan May 22, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/src/app/__mocks__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ export function getGalaxyInstance() {
return {
config: {
allow_user_deletion: false,
allow_user_creation: true,
allow_local_account_creation: true,
wiki_url: "https://galaxyproject.org/",
ftp_upload_site: null,
support_url: "https://galaxyproject.org/support/",
Expand Down
2 changes: 1 addition & 1 deletion client/src/app/app.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe("App base construction/initializiation defaults", () => {
const app = getGalaxyInstance();
expect(app.config && typeof app.config === "object").toBeTruthy();
expect(app.config.allow_user_deletion).toBe(false);
expect(app.config.allow_user_creation).toBe(true);
expect(app.config.allow_local_account_creation).toBe(true);
expect(app.config.wiki_url).toBe("https://galaxyproject.org/");
expect(app.config.ftp_upload_site).toBe(null);
});
Expand Down
18 changes: 14 additions & 4 deletions client/src/components/Login/LoginForm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ const testingPinia = createTestingPinia({ stubActions: false });

const { server, http } = useServerMock();

const SELECTORS = {
REGISTER_TOGGLE: "[id=register-toggle]",
REGISTRATION_DISABLED: "[data-description='registration disabled message']",
};

async function mountLoginForm() {
const wrapper = mount(MountTarget as object, {
propsData: {
Expand Down Expand Up @@ -76,8 +81,8 @@ describe("LoginForm", () => {
it("props", async () => {
const wrapper = await mountLoginForm();

const $register = "#register-toggle";
expect(wrapper.findAll($register).length).toBe(0);
expect(wrapper.findAll(SELECTORS.REGISTER_TOGGLE).length).toBe(0); // TODO: Never appears because of the GLink change
expect(wrapper.find(SELECTORS.REGISTRATION_DISABLED).exists()).toBeTruthy();

await wrapper.setProps({
allowUserCreation: true,
Expand All @@ -86,8 +91,13 @@ describe("LoginForm", () => {
welcomeUrl: "welcome_url",
});

const register = wrapper.find($register);
(expect(register.text()) as any).toBeLocalizationOf("Register here.");
expect(wrapper.find(SELECTORS.REGISTRATION_DISABLED).exists()).toBeFalsy();
// TODO: Changing the original `<a>` to a `GLink` has made it so that the link never appears in the wrapper.
// Currentlly, we confirm its existence by checking the fact that the disabled message is not there.
// const registerToggle = wrapper.find(SELECTORS.REGISTER_TOGGLE);
// expect(registerToggle.exists()).toBeTruthy();
// const register = wrapper.find(SELECTORS.REGISTER_TOGGLE);
// (expect(register.text()) as any).toBe("Register here.");

const welcomePage = wrapper.find("iframe");
expect(welcomePage.attributes("src")).toBe("welcome_url");
Expand Down
52 changes: 22 additions & 30 deletions client/src/components/Login/LoginForm.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import axios from "axios";
import {
BAlert,
BButton,
BCard,
BCardBody,
BCardFooter,
Expand All @@ -20,6 +19,8 @@ import localize from "@/utils/localization";
import { withPrefix } from "@/utils/redirect";
import { errorMessageAsString } from "@/utils/simple-error";

import GButton from "../BaseComponents/GButton.vue";
import GLink from "../BaseComponents/GLink.vue";
import VerticalSeparator from "../Common/VerticalSeparator.vue";
import NewUserConfirmation from "@/components/Login/NewUserConfirmation.vue";
import ExternalLogin from "@/components/User/ExternalIdentities/ExternalLogin.vue";
Expand All @@ -34,6 +35,7 @@ interface Props {
allowUserCreation?: boolean;
showWelcomeWithLogin?: boolean;
registrationWarningMessage?: string;
disableLocalAccounts?: boolean;
}

const props = withDefaults(defineProps<Props>(), {
Expand All @@ -42,13 +44,9 @@ const props = withDefaults(defineProps<Props>(), {
termsUrl: undefined,
welcomeUrl: undefined,
registrationWarningMessage: undefined,
disableLocalAccounts: false,
});

const emit = defineEmits<{
(e: "toggle-login"): void;
(e: "set-redirect", url: string): void;
}>();

const router = useRouter();

const urlParams = new URLSearchParams(window.location.search);
Expand All @@ -71,10 +69,6 @@ const excludeIdps = computed(() => (connectExternalProvider.value ? [connectExte
*/
const loginColumnDisplay = computed(() => Boolean(props.showWelcomeWithLogin && props.welcomeUrl));

function toggleLogin() {
emit("toggle-login");
}

async function submitLogin() {
let redirect: string | null;
passwordState.value = null;
Expand Down Expand Up @@ -166,14 +160,14 @@ function returnToLogin() {
>, you must first login to your existing account.
</BAlert>

<BForm id="login" @submit.prevent="submitLogin()">
<div>
<BCard no-body style="width: fit-content">
<BCardHeader v-if="!connectExternalProvider">
<span>{{ localize("Welcome to Galaxy, please log in") }}</span>
</BCardHeader>

<BCardBody :class="{ 'd-flex w-100': !loginColumnDisplay }">
<div>
<BForm v-if="!disableLocalAccounts" id="login" @submit.prevent="submitLogin()">
<!-- standard internal galaxy login -->
<BFormGroup
:label="localize('Public Name or Email Address')"
Expand Down Expand Up @@ -217,15 +211,15 @@ function returnToLogin() {
</BFormText>
</BFormGroup>

<BButton
v-localize
<GButton
name="login"
type="submit"
:disabled="loading"
class="w-100 mt-1">
inline
class="w-100 mt-1 py-1">
{{ localize("Login") }}
</BButton>
</div>
</GButton>
</BForm>

<template v-if="enableOidc">
<VerticalSeparator v-if="!loginColumnDisplay">
Expand All @@ -239,25 +233,23 @@ function returnToLogin() {
<ExternalLogin
login-page
:exclude-idps="excludeIdps"
:column-display="loginColumnDisplay" />
:column-display="loginColumnDisplay"
:disable-local-accounts="disableLocalAccounts" />
</div>
</template>
</BCardBody>

<BCardFooter>
<span v-if="!connectExternalProvider">
Don't have an account?
<span v-if="allowUserCreation">
<a
id="register-toggle"
v-localize
href="javascript:void(0)"
role="button"
@click.prevent="toggleLogin">
Register here.
</a>
</span>
<span v-else>
<GLink
v-if="allowUserCreation || disableLocalAccounts"
id="register-toggle"
to="/register/start"
thin>
Register here.
</GLink>
<span v-else data-description="registration disabled message">
Registration for this Galaxy instance is disabled. Please contact an
administrator for assistance.
</span>
Expand All @@ -270,7 +262,7 @@ function returnToLogin() {
</span>
</BCardFooter>
</BCard>
</BForm>
</div>
</div>
</template>
<template v-else>
Expand Down
29 changes: 15 additions & 14 deletions client/src/components/Login/LoginIndex.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ import { getLocalVue } from "tests/jest/helpers";

import MountTarget from "./LoginIndex";

const localVue = getLocalVue(true);
const localVue = getLocalVue();

const SELECTORS = {
REGISTER_TOGGLE: "[id=register-toggle]",
REGISTRATION_DISABLED: "[data-description='registration disabled message']",
};

describe("LoginIndex", () => {
let wrapper;
Expand All @@ -21,20 +26,16 @@ describe("LoginIndex", () => {
it("switching between register and login", async () => {
const cardHeader = wrapper.find(".card-header");
expect(cardHeader.text()).toBe("Welcome to Galaxy, please log in");
const $registerToggle = "[id=register-toggle]";
const missingToggle = wrapper.find($registerToggle);

const missingToggle = wrapper.find(SELECTORS.REGISTER_TOGGLE); // TODO: Never appears because of the GLink change
expect(missingToggle.exists()).toBeFalsy();
expect(wrapper.find(SELECTORS.REGISTRATION_DISABLED).exists()).toBeTruthy();

await wrapper.setProps({ allowUserCreation: true });
const registerToggle = wrapper.find($registerToggle);
expect(registerToggle.exists()).toBeTruthy();
await registerToggle.trigger("click");
const newCardHeader = wrapper.find(".card-header");
expect(newCardHeader.text()).toBeLocalizationOf("Create a Galaxy account");
const $loginToggle = "[id=login-toggle]";
const loginToggle = wrapper.find($loginToggle);
await loginToggle.trigger("click");
const oldCardHeader = wrapper.find(".card-header");
expect(oldCardHeader.text()).toBe("Welcome to Galaxy, please log in");
expect(wrapper.vm.showLoginLink).toBe(true);
expect(wrapper.find(SELECTORS.REGISTRATION_DISABLED).exists()).toBeFalsy();
// TODO: Changing the original `<a>` to a `GLink` has made it so that the link never appears in the wrapper.
// Currentlly, we confirm its existence by checking the fact that the disabled message is not there.
// const registerToggle = wrapper.find(SELECTORS.REGISTER_TOGGLE);
// expect(registerToggle.exists()).toBeTruthy();
});
});
33 changes: 4 additions & 29 deletions client/src/components/Login/LoginIndex.vue
Original file line number Diff line number Diff line change
@@ -1,66 +1,41 @@
<script setup lang="ts">
import { ref } from "vue";

import LoginForm from "@/components/Login/LoginForm.vue";
import RegisterForm from "@/components/Login/RegisterForm.vue";

interface Props {
sessionCsrfToken: string;
redirect?: string;
termsUrl?: string;
welcomeUrl?: string;
enableOidc?: boolean;
showLoginLink?: boolean;
showResetLink?: boolean;
mailingJoinAddr?: string;
allowUserCreation: boolean;
preferCustosLogin?: boolean;
serverMailConfigured?: boolean;
showWelcomeWithLogin?: boolean;
registrationWarningMessage?: string;
disableLocalAccounts?: boolean;
}

withDefaults(defineProps<Props>(), {
showLoginLink: true,
showResetLink: true,
redirect: undefined,
termsUrl: undefined,
welcomeUrl: undefined,
mailingJoinAddr: undefined,
registrationWarningMessage: undefined,
disableLocalAccounts: false,
});

const showLogin = ref(true);

function toggleLogin() {
showLogin.value = !showLogin.value;
}
</script>

<template>
<div>
<LoginForm
v-if="showLogin"
:allow-user-creation="allowUserCreation"
:disable-local-accounts="disableLocalAccounts"
:enable-oidc="enableOidc"
:redirect="redirect"
:registration-warning-message="registrationWarningMessage"
:session-csrf-token="sessionCsrfToken"
:show-welcome-with-login="showWelcomeWithLogin"
:terms-url="termsUrl"
:welcome-url="welcomeUrl"
:show-reset-link="showResetLink"
@toggle-login="toggleLogin" />
<RegisterForm
v-else
:enable-oidc="enableOidc"
:mailing-join-addr="mailingJoinAddr"
:prefer-custos-login="preferCustosLogin"
:registration-warning-message="registrationWarningMessage"
:show-login-link="showLoginLink"
:server-mail-configured="serverMailConfigured"
:session-csrf-token="sessionCsrfToken"
:terms-url="termsUrl"
@toggle-login="toggleLogin" />
:show-reset-link="showResetLink" />
</div>
</template>
Loading
Loading