mirrors/hedgedoc
1
0
Fork 0
mirror of https://github.com/hedgedoc/hedgedoc.git synced 2025-06-06 01:21:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix(auth): guest login fixes, more reliable pending user session handler

Browse source 
Signed-off-by: Erik Michelson <github@erik.michelson.eu>
This commit is contained in:
Erik Michelson 2025-05-28 22:53:23 +00:00
parent 167135a8d0
commit 3a90c9ca96
No known key found for this signature in database
GPG key ID: DB99ADDDC5C0AF82
9 changed files with 147 additions and 97 deletions
Download patch file Download diff file Expand all files Collapse all files

37
backend/src/api/private/auth/auth.controller.ts
View file

@ -68,14 +68,10 @@ export class AuthController {
getPendingUserData(
@Req() request: RequestWithSession,
): Partial<PendingUserInfoDto> {
if (
!request.session.newUserData ||
!request.session.authProviderIdentifier ||
!request.session.authProviderType
) {
if (!request.session.pendingUser?.confirmationData) {
throw new BadRequestException('No pending user data');
}
return request.session.newUserData;
return request.session.pendingUser.confirmationData;
}
@Put('pending-user')
@ -85,32 +81,33 @@ export class AuthController {
@Body() pendingUserConfirmationData: PendingUserConfirmationDto,
): Promise<void> {
if (
!request.session.newUserData ||
!request.session.authProviderIdentifier ||
!request.session.authProviderType ||
!request.session.providerUserId
!request.session.pendingUser?.confirmationData ||
!request.session.pendingUser?.authProviderType ||
!request.session.pendingUser?.authProviderIdentifier ||
!request.session.pendingUser?.providerUserId
) {
throw new BadRequestException('No pending user data');
}
request.session.userId =
await this.identityService.createUserWithIdentityFromPendingUserConfirmation(
request.session.newUserData,
request.session.pendingUser.confirmationData,
pendingUserConfirmationData,
request.session.authProviderType,
request.session.authProviderIdentifier,
request.session.providerUserId,
request.session.pendingUser.authProviderType,
request.session.pendingUser.authProviderIdentifier,
request.session.pendingUser.providerUserId,
);
request.session.authProviderType =
request.session.pendingUser.authProviderType;
request.session.authProviderIdentifier =
request.session.pendingUser.authProviderIdentifier;
// Cleanup
request.session.newUserData = undefined;
request.session.pendingUser = undefined;
}
@Delete('pending-user')
@OpenApi(204, 400)
deletePendingUserData(@Req() request: RequestWithSession): void {
request.session.newUserData = undefined;
request.session.authProviderIdentifier = undefined;
request.session.authProviderType = undefined;
request.session.providerUserId = undefined;
request.session.oidcIdToken = undefined;
request.session.pendingUser = undefined;
request.session.oidc = undefined;
}
}

12
backend/src/api/private/auth/ldap/ldap.controller.ts
View file

@ -54,9 +54,6 @@ export class LdapController {
loginDto.password,
);
try {
request.session.authProviderType = AuthProviderType.LDAP;
request.session.authProviderIdentifier = ldapIdentifier;
request.session.providerUserId = userInfo.id;
const identity =
await this.identityService.getIdentityFromUserIdAndProviderType(
userInfo.id,
@ -71,11 +68,18 @@ export class LdapController {
userInfo.photoUrl,
);
}
request.session.authProviderType = AuthProviderType.LDAP;
request.session.authProviderIdentifier = ldapIdentifier;
request.session.userId = identity[FieldNameIdentity.userId];
return { newUser: false };
} catch (error) {
if (error instanceof NotInDBError) {
request.session.newUserData = userInfo;
request.session.pendingUser = {
authProviderType: AuthProviderType.LDAP,
authProviderIdentifier: ldapIdentifier,
confirmationData: userInfo,
providerUserId: userInfo.id,
};
return { newUser: true };
}
this.logger.error(`Error during LDAP login: ${String(error)}`);

2
backend/src/api/private/auth/local/local.controller.ts
View file

@ -59,6 +59,7 @@ export class LocalController {
// Log the user in after registration
request.session.authProviderType = AuthProviderType.LOCAL;
request.session.userId = userId;
request.session.pendingUser = undefined;
}
@UseGuards(LoginEnabledGuard, SessionGuard)
@ -98,6 +99,7 @@ export class LocalController {
);
request.session.userId = identity[FieldNameIdentity.userId];
request.session.authProviderType = AuthProviderType.LOCAL;
request.session.pendingUser = undefined;
} catch (error) {
this.logger.log(`Failed to log in user: ${String(error)}`, 'login');
throw new UnauthorizedException('Invalid username or password');

19
backend/src/api/private/auth/oidc/oidc.controller.ts
View file

@ -45,10 +45,14 @@ export class OidcController {
): { url: string } {
const code = this.oidcService.generateCode();
const state = this.oidcService.generateState();
request.session.oidcLoginCode = code;
request.session.oidcLoginState = state;
request.session.authProviderType = AuthProviderType.OIDC;
request.session.authProviderIdentifier = oidcIdentifier;
request.session.oidc = {
loginCode: code,
loginState: state,
};
request.session.pendingUser = {
authProviderType: AuthProviderType.OIDC,
authProviderIdentifier: oidcIdentifier,
};
const authorizationUrl = this.oidcService.getAuthorizationUrl(
oidcIdentifier,
code,
@ -69,12 +73,11 @@ export class OidcController {
oidcIdentifier,
request,
);
const oidcUserIdentifier = request.session.providerUserId;
const oidcUserIdentifier = request.session.pendingUser?.providerUserId;
if (!oidcUserIdentifier) {
this.logger.log('No OIDC user identifier in callback', 'callback');
throw new UnauthorizedException('No OIDC user identifier found');
}
request.session.authProviderType = AuthProviderType.OIDC;
const identity = await this.oidcService.getExistingOidcIdentity(
oidcIdentifier,
oidcUserIdentifier,
@ -82,7 +85,6 @@ export class OidcController {
const mayUpdate = this.identityService.mayUpdateIdentity(oidcIdentifier);
if (identity === null) {
request.session.newUserData = userInfo;
return { url: '/new-user' };
}
@ -97,6 +99,9 @@ export class OidcController {
}
request.session.userId = userId;
request.session.authProviderType = AuthProviderType.OIDC;
request.session.authProviderIdentifier = oidcIdentifier;
request.session.pendingUser = undefined;
return { url: '/' };
} catch (error) {
if (error instanceof HttpException) {

73
backend/src/auth/oidc/oidc.service.ts
View file

@ -69,16 +69,18 @@ export class OidcService {
}
/**
* @async
* Fetches the client and its config (issuer, metadata) for the given OIDC configuration.
* Fetches the client and its config (issuer, metadata) for the given OIDC configuration
*
* @param {OidcConfig} oidcConfig The OIDC configuration to fetch the client config for
* @returns {OidcClientConfigEntry} A promise that resolves to the client configuration.
* @param oidcConfig The OIDC configuration to fetch the client config for
* @returns A promise that resolves to the client configuration.
*/
private async fetchClientConfig(
oidcConfig: OidcConfig,
): Promise<OidcClientConfigEntry> {
const useAutodiscover = oidcConfig.authorizeUrl === undefined;
const useAutodiscover =
oidcConfig.authorizeUrl === undefined ||
oidcConfig.tokenUrl === undefined ||
oidcConfig.userinfoUrl === undefined;
const issuer = useAutodiscover
? await Issuer.discover(oidcConfig.issuer)
: new Issuer({
@ -117,7 +119,7 @@ export class OidcService {
/**
* Generates a secure code verifier for the OIDC login.
*
* @returns {string} The generated code verifier.
* @returns The generated code verifier.
*/
generateCode(): string {
return generators.codeVerifier();
@ -126,7 +128,7 @@ export class OidcService {
/**
* Generates a random state for the OIDC login.
*
* @returns {string} The generated state.
* @returns The generated state.
*/
generateState(): string {
return generators.state();
@ -135,10 +137,10 @@ export class OidcService {
/**
* Generates the authorization URL for the given OIDC identifier and code.
*
* @param {string} oidcIdentifier The identifier of the OIDC configuration
* @param {string} code The code verifier generated for the login
* @param {string} state The state generated for the login
* @returns {string} The generated authorization URL
* @param oidcIdentifier The identifier of the OIDC configuration
* @param code The code verifier generated for the login
* @param state The state generated for the login
* @returns The generated authorization URL
*/
getAuthorizationUrl(
oidcIdentifier: string,
@ -163,7 +165,6 @@ export class OidcService {
}
/**
* @async
* Extracts the user information from the callback and stores them in the session.
* Afterward, the user information is returned.
*
@ -184,8 +185,8 @@ export class OidcService {
const client = clientConfig.client;
const oidcConfig = clientConfig.config;
const params = client.callbackParams(request);
const code = request.session.oidcLoginCode;
const state = request.session.oidcLoginState;
const code = request.session.oidc?.loginCode;
const state = request.session.oidc?.loginState;
const isAutodiscovered = clientConfig.config.authorizeUrl === undefined;
const callbackMethod = isAutodiscovered
? client.callback.bind(client)
@ -196,7 +197,9 @@ export class OidcService {
state,
});
request.session.oidcIdToken = tokenSet.id_token;
request.session.oidc = {
idToken: tokenSet.id_token,
};
const userInfoResponse = await client.userinfo(tokenSet);
const userId = OidcService.getResponseFieldValue(
userInfoResponse,
@ -229,22 +232,21 @@ export class OidcService {
photoUrl: photoUrl ?? null,
email: email ?? null,
};
request.session.providerUserId = userId;
request.session.newUserData = newUserData;
// Cleanup: The code isn't necessary anymore
request.session.oidcLoginCode = undefined;
request.session.oidcLoginState = undefined;
request.session.pendingUser = {
authProviderType: AuthProviderType.OIDC,
authProviderIdentifier: oidcIdentifier,
providerUserId: userId,
confirmationData: newUserData,
};
return newUserData;
}
/**
* @async
* Checks if an identity exists for a given OIDC user and returns it if it does.
* Checks if an identity exists for a given OIDC user and returns it if it does
*
* @param {string} oidcIdentifier The identifier of the OIDC configuration
* @param {string} oidcUserId The id of the user in the OIDC system
* @returns {Identity} The identity if it exists
* @returns {null} when the identity does not exist
* @param oidcIdentifier The identifier of the OIDC configuration
* @param oidcUserId The id of the user in the OIDC system
* @returns The identity if it exists, null otherwise
*/
async getExistingOidcIdentity(
oidcIdentifier: string,
@ -277,11 +279,10 @@ export class OidcService {
}
/**
* Returns the logout URL for the given request if the user is logged in with OIDC.
* Returns the logout URL for the given request if the user is logged in with OIDC
*
* @param {RequestWithSession} request The request containing the session
* @returns {string} The logout URL if the user is logged in with OIDC
* @returns {null} when there is no logout URL to redirect to
* @param request The request containing the session
* @returns The logout URL if the user is logged in with OIDC, or null if there is no URL to redirect to
*/
getLogoutUrl(request: RequestWithSession): string | null {
const oidcIdentifier = request.session.authProviderIdentifier;
@ -296,7 +297,7 @@ export class OidcService {
}
const issuer = clientConfig.issuer;
const endSessionEndpoint = issuer.metadata.end_session_endpoint;
const idToken = request.session.oidcIdToken;
const idToken = request.session.oidc?.idToken;
if (!endSessionEndpoint) {
return null;
}
@ -304,12 +305,12 @@ export class OidcService {
}
/**
* Returns a specific field from the userinfo object or a default value.
* Returns a specific field from the userinfo object or a default value
*
* @param {UserinfoResponse} response The response from the OIDC userinfo endpoint
* @param {string} field The field to get from the response
* @param {string|undefined} defaultValue The default value to return if the value is empty
* @returns {string|undefined} The value of the field from the response or the default value
* @param response The response from the OIDC userinfo endpoint
* @param field The field to get from the response
* @param defaultValue The default value to return if the value is empty
* @returns The value of the field from the response or the default value
*/
private static getResponseFieldValue<T extends string | undefined>(
response: UserinfoResponse,

36
backend/src/database/migrations/20250312211152_initial.js
View file

@ -1,3 +1,9 @@
/*
* SPDX-FileCopyrightText: 2025 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
/* eslint-disable */
const {
AuthProviderType,
@ -83,7 +89,8 @@ const up = async function (knex) {
.unsigned()
.notNullable()
.references(FieldNameUser.id)
.inTable(TableUser);
.inTable(TableUser)
.onDelete('CASCADE');
});
// Create aliases table
@ -97,7 +104,8 @@ const up = async function (knex) {
.unsigned()
.notNullable()
.references(FieldNameNote.id)
.inTable(TableNote);
.inTable(TableNote)
.onDelete('CASCADE');
table.boolean(FieldNameAlias.isPrimary).nullable();
table.unique([FieldNameAlias.noteId, FieldNameAlias.isPrimary], {
indexName: 'only_one_note_can_be_primary',
@ -113,7 +121,8 @@ const up = async function (knex) {
.unsigned()
.notNullable()
.references(FieldNameUser.id)
.inTable(TableUser);
.inTable(TableUser)
.onDelete('CASCADE');
table.string(FieldNameApiToken.label).notNullable();
table.string(FieldNameApiToken.secretHash).notNullable();
table
@ -130,7 +139,8 @@ const up = async function (knex) {
.unsigned()
.notNullable()
.references(FieldNameUser.id)
.inTable(TableUser);
.inTable(TableUser)
.onDelete('CASCADE');
table.enu(
FieldNameIdentity.providerType,
[AuthProviderType.LDAP, AuthProviderType.LOCAL, AuthProviderType.OIDC], // AuthProviderType.GUEST is not relevant for the DB
@ -168,13 +178,15 @@ const up = async function (knex) {
.unsigned()
.notNullable()
.references(FieldNameUser.id)
.inTable(TableUser);
.inTable(TableUser)
.onDelete('CASCADE');
table
.integer(FieldNameGroupUser.groupId)
.unsigned()
.notNullable()
.references(FieldNameGroup.id)
.inTable(TableGroup);
.inTable(TableGroup)
.onDelete('CASCADE');
table.primary([FieldNameGroupUser.userId, FieldNameGroupUser.groupId]);
});
@ -186,7 +198,8 @@ const up = async function (knex) {
.unsigned()
.notNullable()
.references(FieldNameNote.id)
.inTable(TableNote);
.inTable(TableNote)
.onDelete('CASCADE');
table.text(FieldNameRevision.patch).notNullable();
table.text(FieldNameRevision.content).notNullable();
table.string(FieldNameRevision.title).notNullable();
@ -303,7 +316,8 @@ const up = async function (knex) {
.unsigned()
.nullable()
.references(FieldNameNote.id)
.inTable(TableNote);
.inTable(TableNote)
.onDelete('SET NULL');
table
.integer(FieldNameMediaUpload.userId)
.unsigned()
@ -340,13 +354,15 @@ const up = async function (knex) {
.unsigned()
.notNullable()
.references(FieldNameUser.id)
.inTable(TableUser);
.inTable(TableUser)
.onDelete('CASCADE');
table
.integer(FieldNameUserPinnedNote.noteId)
.unsigned()
.notNullable()
.references(FieldNameNote.id)
.inTable(TableNote);
.inTable(TableNote)
.onDelete('CASCADE');
table.primary([
FieldNameUserPinnedNote.userId,
FieldNameUserPinnedNote.noteId,

44
backend/src/sessions/session-state.type.ts
View file

@ -7,6 +7,31 @@ import { AuthProviderType, PendingUserInfoDto } from '@hedgedoc/commons';
import { FieldNameUser, User } from '@hedgedoc/database';
import { Cookie } from 'express-session';
interface OidcAuthSessionState {
/** The id token to identify a user session with an OIDC auth provider, required for the logout */
idToken?: string;
/** The (random) OIDC code for verifying that OIDC responses match the OIDC requests */
loginCode?: string;
/** The (random) OIDC state for verifying that OIDC responses match the OIDC requests */
loginState?: string;
}
interface PendingUserSessionState {
/** The pending user confirmation data */
confirmationData?: PendingUserInfoDto;
/** The pending user auth provider type */
authProviderType?: AuthProviderType;
/** The pending user auth provider identifier */
authProviderIdentifier?: string;
/** The pending user id as provided from the external auth provider, required for matching to a HedgeDoc identity */
providerUserId?: string;
}
export interface SessionState {
/** Details about the currently used session cookie */
cookie: Cookie;
@ -14,24 +39,15 @@ export interface SessionState {
/** Contains the username if logged in completely, is undefined when not being logged in */
userId?: User[FieldNameUser.id];
/** The auth provider that is used for the current login or pending login */
/** The auth provider that is used for the current login */
authProviderType?: AuthProviderType;
/** The identifier of the auth provider that is used for the current login or pending login */
/** The identifier of the auth provider that is used for the current login */
authProviderIdentifier?: string;
/** The id token to identify a user session with an OIDC auth provider, required for the logout */
oidcIdToken?: string;
/** The (random) OIDC code for verifying that OIDC responses match the OIDC requests */
oidcLoginCode?: string;
/** The (random) OIDC state for verifying that OIDC responses match the OIDC requests */
oidcLoginState?: string;
/** The user id as provided from the external auth provider, required for matching to a HedgeDoc identity */
providerUserId?: string;
/** Session data used on OIDC login */
oidc?: OidcAuthSessionState;
/** The user data of the user that is currently being created */
newUserData?: PendingUserInfoDto;
pendingUser?: PendingUserSessionState;
}

4
frontend/src/api/me/index.ts
View file

@ -5,8 +5,8 @@
*/
import { DeleteApiRequestBuilder } from '../common/api-request-builder/delete-api-request-builder'
import { GetApiRequestBuilder } from '../common/api-request-builder/get-api-request-builder'
import { PostApiRequestBuilder } from '../common/api-request-builder/post-api-request-builder'
import type { UpdateUserInfoDto, LoginUserInfoDto, MediaUploadDto } from '@hedgedoc/commons'
import { PutApiRequestBuilder } from '../common/api-request-builder/put-api-request-builder'
/**
* Returns metadata about the currently signed-in user from the API.
@ -36,7 +36,7 @@ export const deleteUser = async (): Promise<void> => {
* @throws {Error} when the api request wasn't successful.
*/
export const updateUser = async (displayName: string | null, email: string | null): Promise<void> => {
await new PostApiRequestBuilder<void, UpdateUserInfoDto>('me/profile')
await new PutApiRequestBuilder<void, UpdateUserInfoDto>('me/profile')
.withJsonBody({
displayName,
email

17
frontend/src/components/application-loader/initializers/login-or-register-guest.ts
View file

@ -7,6 +7,9 @@
import { logInGuest, registerGuest } from '../../../api/auth/guest'
import { store } from '../../../redux'
import { fetchAndSetUser } from '../../login-page/utils/fetch-and-set-user'
import { Logger } from '../../../utils/logger'
const logger = new Logger('LoginOrRegisterGuest')
/**
* Handles the auth process towards the backend for guests
@ -14,18 +17,24 @@ import { fetchAndSetUser } from '../../login-page/utils/fetch-and-set-user'
* If there is a guest uuid in local storage, the guest with that uuid is logged in.
* If there is no guest uuid in local storage, a new guest is registered and logged in.
* The uuid is stored in local storage afterward.
*
* @param ignoreSavedUuid If true, the function will not check for a saved guest uuid in local storage
*/
export const loginOrRegisterGuest = async (): Promise<void> => {
export const loginOrRegisterGuest = async (ignoreSavedUuid?: boolean): Promise<void> => {
const userState = store.getState().user
if (userState !== null) {
return
}
const guestUuid = window.localStorage.getItem('guestUuid')
const guestUuid = ignoreSavedUuid ? null : window.localStorage.getItem('guestUuid')
if (guestUuid === null) {
const { uuid } = await registerGuest()
window.localStorage.setItem('guestUuid', uuid)
return
}
await logInGuest(guestUuid)
await fetchAndSetUser()
logInGuest(guestUuid)
.then(fetchAndSetUser)
.catch((error: unknown) => {
logger.error('Error logging in guest user', error)
return loginOrRegisterGuest(true)
})
}