Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
6e20c6c
fix: 🐛 properly show username for oidc auth method in user menu
lisbet-alvarez Sep 11, 2025
615977d
test: 💍 fix admin ui tests
lisbet-alvarez Sep 18, 2025
1761407
test: 💍 fix desktop ui tests
lisbet-alvarez Sep 18, 2025
1f57d7a
Merge branch 'main' into ICU-17231-bug-fix-oidc-authentications-not-p…
lisbet-alvarez Sep 25, 2025
2be6f2b
refactor: 💡 admin to not store username in authenticated object
lisbet-alvarez Oct 7, 2025
a47e9d2
refactor: 💡 use getter in app controller
lisbet-alvarez Oct 7, 2025
5e8963f
test: 💍 POC for how we could update our tests to use auth accou
lisbet-alvarez Oct 8, 2025
4fafec9
Merge branch 'ICU-17231-bug-fix-oidc-authentications-not-properly-sho…
lisbet-alvarez Oct 8, 2025
2d09982
refactor: 💡 remove unused service
lisbet-alvarez Oct 8, 2025
d2aeaf3
test: 💍 fix all tests in ui/admin
lisbet-alvarez Oct 10, 2025
0db4551
test: 💍 revert one change
lisbet-alvarez Oct 10, 2025
2a60e00
refactor: 💡 update ui/desktop w/ feedback + fix tests
lisbet-alvarez Oct 13, 2025
41dc077
refactor: 💡 add in error handling
lisbet-alvarez Oct 15, 2025
0de4f27
refactor: 💡 cleanup unused service
lisbet-alvarez Oct 16, 2025
c6cca0e
Merge branch 'main' into ICU-17231-bug-fix-oidc-authentications-not-p…
lisbet-alvarez Oct 28, 2025
6e2adb5
test: 💍 fix e2e test selector
lisbet-alvarez Oct 28, 2025
72a0ece
Merge branch 'ICU-17231-bug-fix-oidc-authentications-not-properly-sho…
lisbet-alvarez Oct 28, 2025
5bdf971
Merge branch 'main' into ICU-17231-bug-fix-oidc-authentications-not-p…
lisbet-alvarez Oct 30, 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
18 changes: 18 additions & 0 deletions addons/api/mirage/factories/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { trait } from 'miragejs';
import permissions from '../helpers/permissions';
import generateId from '../helpers/id';
import { faker } from '@faker-js/faker';
import { TYPE_AUTH_METHOD_PASSWORD } from 'api/models/auth-method';

export default factory.extend({
type: 'global',
Expand Down Expand Up @@ -108,4 +109,21 @@ export default factory.extend({
}
},
}),

withGlobalAuth: trait({
afterCreate(record, server) {
if (record.type === 'global') {
const authMethod = server.create('auth-method', {
type: TYPE_AUTH_METHOD_PASSWORD,
scope: record,
});
server.create('account', {
type: TYPE_AUTH_METHOD_PASSWORD,
full_name: 'admin',
scope: record,
authMethod,
});
}
},
}),
});
22 changes: 14 additions & 8 deletions addons/api/mirage/route-handlers/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const oidcRequiredAttempts = 3;

const commandHandlers = {
password: {
login: (payload, scopeAttrs) => {
login: (payload, auth_method_id, account_id, scopeAttrs) => {
if (payload.attributes.login_name === 'error') {
return new Response(400);
} else {
Expand All @@ -25,9 +25,9 @@ const commandHandlers = {
scope: scopeAttrs,
id: 'token123',
token: 'thetokenstring',
account_id: 'authenticatedaccount',
account_id: account_id || 'authenticatedaccount',
user_id: 'authenticateduser',
auth_method_id: 'authmethod123',
auth_method_id: auth_method_id || 'authmethod123',
created_time: '',
updated_time: '',
last_used_time: '',
Expand Down Expand Up @@ -61,7 +61,7 @@ const commandHandlers = {
},
},
),
token: (_, scopeAttrs) => {
token: (_, auth_method_id, account_id, scopeAttrs) => {
oidcAttemptCounter++;
if (oidcAttemptCounter < oidcRequiredAttempts) {
return new Response(202);
Expand All @@ -74,9 +74,9 @@ const commandHandlers = {
scope: scopeAttrs,
id: 'token123',
token: 'thetokenstring',
account_id: '1',
account_id: account_id || '1',
user_id: 'authenticateduser',
auth_method_id: 'authmethod123',
auth_method_id: auth_method_id || 'authmethod123',
created_time: '',
updated_time: '',
last_used_time: '',
Expand All @@ -90,7 +90,7 @@ const commandHandlers = {
};

// Handles all custom methods on /auth-methods/:id route
export function authHandler({ scopes, authMethods }, request) {
export function authHandler({ scopes, authMethods, accounts }, request) {
const [, id, method] = request.params.id_method.match(
/(?<id>.[^:]*):(?<method>(.*))/,
);
Expand All @@ -100,8 +100,14 @@ export function authHandler({ scopes, authMethods }, request) {
const payload = JSON.parse(request.requestBody);
const { command } = payload;
const scope = scopes.find(authMethod.scopeId);
const account = accounts.where({ authMethodId: authMethod.id })?.models[0];
const scopeAttrs = this.serialize(scopes.find(scope.id));
return commandHandlers[authMethod.type][command](payload, scopeAttrs);
return commandHandlers[authMethod.type][command](
payload,
authMethod.id,
account?.id,
scopeAttrs,
);
}

// TODO: this handler doesn't really belong here, but we already route
Expand Down
3 changes: 1 addition & 2 deletions addons/auth/addon/authenticators/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,14 @@ export default class BaseAuthenticator extends SimpleAuthBaseAuthenticator {
* @param {string} username
* @return {object}
*/
normalizeData(data, username) {
normalizeData(data) {
// Pull fields up from `data.attributes` for easier access in JavaScript.
// The `attributes` field exists on the Go side for its convenience but is
// unnecessary here.
Object.assign(data, data.attributes);
// Add booleans indicated the scope type
data.isGlobal = data?.scope?.type === 'global';
data.isOrg = data?.scope?.type === 'org';
if (username) data.username = username;
return data;
}

Expand Down
4 changes: 1 addition & 3 deletions addons/auth/addon/authenticators/password.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ export default class PasswordAuthenticator extends BaseAuthenticator {
fetch(authEndpointURL, { method: 'post', body }),
);
const json = await response.json();
return response.status < 400
? resolve(this.normalizeData(json, login_name))
: reject();
return response.status < 400 ? resolve(this.normalizeData(json)) : reject();
}
}
1 change: 1 addition & 0 deletions addons/core/translations/en-us.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ titles:
project-actions: Project Actions
user-menu: User Menu
back-link: 'Back to {scope}'
authenticated: Signed in as
descriptions:
empty-set: There are no items to display yet. You may be able to add items or try back later.
cluster-url-initialization: To get started, please enter your cluster URL
Expand Down
4 changes: 1 addition & 3 deletions e2e-tests/admin/tests/auth-method-oidc-vault.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,7 @@ test(
).toBeVisible();

// Log back in as an admin
await page.getByRole('button', { name: 'User Menu' }).click();
await page.getByRole('button', { name: 'Sign Out' }).click();
await expect(page.getByRole('button', { name: 'Sign In' })).toBeVisible();
await loginPage.logout(email);
await loginPage.login(adminLoginName, adminPassword);
await expect(
page.getByRole('navigation', { name: 'breadcrumbs' }).getByText('Orgs'),
Expand Down
2 changes: 2 additions & 0 deletions ui/admin/app/routes/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ export default class ApplicationRoute extends Route {
if (userId && hostUrl) {
await this.sqlite.setup(formatDbName(userId, hostUrl));
}

await this.session.loadAuthenticatedAccount();
}
}

Expand Down
27 changes: 27 additions & 0 deletions ui/admin/app/services/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@

import BaseSessionService from 'ember-simple-auth/services/session';
import { service } from '@ember/service';
import { tracked } from '@glimmer/tracking';
import { formatDbName } from 'api/services/sqlite';

export default class SessionService extends BaseSessionService {
@service sqlite;
@service('browser/window') window;
@service store;

@tracked username;

/**
* Extend ember simple auth's handleAuthentication method
Expand All @@ -22,8 +26,31 @@ export default class SessionService extends BaseSessionService {
await this.sqlite.setup(formatDbName(userId, hostUrl));
}

await this.loadAuthenticatedAccount();

// We let ember-simple-auth handle transitioning back to the index after authentication.
// This route can be configured in our environment config.
super.handleAuthentication(...arguments);
}

/**
* Loads account used to authenticate so that it can be used to display
* the authenticated username.
*/
async loadAuthenticatedAccount() {
if (this.data?.authenticated?.account_id) {
try {
const account = await this.store.findRecord(
'account',
this.data.authenticated.account_id,
);
this.username = account.accountName;
} catch (e) {
// We are purposefully ignoring errors since loading the
// authenticated account should not block a user because
// account information is for populating a username.
console.error(e);
}
}
}
}
15 changes: 15 additions & 0 deletions ui/admin/app/styles/app.scss
Original file line number Diff line number Diff line change
Expand Up @@ -1025,3 +1025,18 @@
.injected-app-credential-alert {
margin-bottom: 1rem;
}

// User menu dropdown in global side nav
.user-menu-dropdown {
max-width: 125px;

&__toggle-button {
width: 100%;

.hds-dropdown-toggle-button__text {
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
}
}
}
94 changes: 63 additions & 31 deletions ui/admin/app/templates/application.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -71,41 +71,73 @@
</Hds::Dropdown>
{{/if}}

<Hds::Dropdown
@enableCollisionDetection={{true}}
data-test-side-nav-user-dropdown
as |dd|
>
{{#if this.session.data.authenticated.username}}
{{#if this.session.username}}
<Hds::Dropdown
@enableCollisionDetection={{true}}
class='user-menu-dropdown'
data-test-side-nav-user-dropdown
as |dd|
>
<dd.ToggleButton
@icon='user'
@text={{this.session.data.authenticated.username}}
@text={{this.session.username}}
{{hds-tooltip this.session.username}}
class='user-menu-dropdown__toggle-button'
/>
{{else}}
<dd.Title @text={{t 'titles.authenticated'}} />
<dd.Description @text={{this.session.username}} />
<dd.Separator />

<dd.Interactive @route='account.change-password'>{{t
'actions.change-password'
}}</dd.Interactive>
<dd.Separator />

<dd.Interactive {{on 'click' this.invalidateSession}}>{{t
'actions.deauthenticate'
}}</dd.Interactive>
<dd.Separator />

<dd.Title @text={{t 'actions.toggle-color-theme'}} />
{{#each this.themes as |theme|}}
<dd.Radio
@value={{theme.value}}
checked={{eq this.session.data.theme theme.value}}
{{on 'change' (fn this.toggleTheme theme.value)}}
>
{{t (concat 'themes.' theme.label)}}
</dd.Radio>
{{/each}}
</Hds::Dropdown>
{{else}}
<Hds::Dropdown
@enableCollisionDetection={{true}}
data-test-side-nav-user-dropdown
as |dd|
>
<dd.ToggleIcon @icon='user' @text={{t 'titles.user-menu'}} />
{{/if}}

<dd.Interactive @route='account.change-password'>{{t
'actions.change-password'
}}</dd.Interactive>
<dd.Separator />

<dd.Interactive {{on 'click' this.invalidateSession}}>{{t
'actions.deauthenticate'
}}</dd.Interactive>
<dd.Separator />

<dd.Title @text={{t 'actions.toggle-color-theme'}} />
{{#each this.themes as |theme|}}
<dd.Radio
@value={{theme.value}}
checked={{eq this.session.data.theme theme.value}}
{{on 'change' (fn this.toggleTheme theme.value)}}
>
{{t (concat 'themes.' theme.label)}}
</dd.Radio>
{{/each}}
</Hds::Dropdown>
<dd.Interactive @route='account.change-password'>{{t
'actions.change-password'
}}</dd.Interactive>
<dd.Separator />

<dd.Interactive {{on 'click' this.invalidateSession}}>{{t
'actions.deauthenticate'
}}</dd.Interactive>
<dd.Separator />

<dd.Title @text={{t 'actions.toggle-color-theme'}} />
{{#each this.themes as |theme|}}
<dd.Radio
@value={{theme.value}}
checked={{eq this.session.data.theme theme.value}}
{{on 'change' (fn this.toggleTheme theme.value)}}
>
{{t (concat 'themes.' theme.label)}}
</dd.Radio>
{{/each}}
</Hds::Dropdown>
{{/if}}
</:actions>
</Hds::SideNav::Header>
</:header>
Expand Down
14 changes: 2 additions & 12 deletions ui/admin/tests/acceptance/accounts/change-password-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,19 @@
import { module, test } from 'qunit';
import { visit, currentURL, click, fillIn } from '@ember/test-helpers';
import { setupApplicationTest } from 'admin/tests/helpers';
import setupMirage from 'ember-cli-mirage/test-support/setup-mirage';
import { setupSqlite } from 'api/test-support/helpers/sqlite';
import { Response } from 'miragejs';
import {
authenticateSession,
invalidateSession,
} from 'ember-simple-auth/test-support';
import { invalidateSession } from 'ember-simple-auth/test-support';
import * as selectors from './selectors';
import * as commonSelectors from 'admin/tests/helpers/selectors';
import { setRunOptions } from 'ember-a11y-testing/test-support';

module('Acceptance | accounts | change password', function (hooks) {
setupApplicationTest(hooks);
setupMirage(hooks);
setupSqlite(hooks);

const instances = {
scopes: {
global: null,
org: null,
},
account: null,
Expand All @@ -35,7 +29,6 @@ module('Acceptance | accounts | change password', function (hooks) {
};

hooks.beforeEach(async function () {
instances.scopes.global = this.server.create('scope', { id: 'global' });
instances.scopes.org = this.server.create('scope', {
type: 'org',
scope: { id: 'global', type: 'global' },
Expand All @@ -46,10 +39,7 @@ module('Acceptance | accounts | change password', function (hooks) {
instances.account = this.server.create('account', {
scope: instances.scopes.org,
});
await authenticateSession({
account_id: instances.account.id,
username: 'admin',
});

// Generate route URLs for resources
urls.orgScope = `/scopes/${instances.scopes.org.id}`;
urls.changePassword = `/account/change-password`;
Expand Down
Loading
Loading