Skip to content

Commit

Permalink
[auth] fix missing updates to dynamic login providers
Browse files Browse the repository at this point in the history
this primarily affects self-hosted installation where the `ownerId` is about to change during the initial setup of the login/git provider.
  • Loading branch information
AlexTugarev committed Mar 31, 2022
1 parent 9ee97f3 commit e8020f4
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class AuthProviderEntryDBImpl implements AuthProviderEntryDB {
async storeAuthProvider(ap: AuthProviderEntry, updateOAuthRevision: boolean): Promise<AuthProviderEntry> {
const repo = await this.getAuthProviderRepo();
if (updateOAuthRevision) {
(ap.oauthRevision as any) = this.oauthContentHash(ap.oauth);
(ap.oauthRevision as any) = this.oauthContentHash(ap);
}
return repo.save(ap);
}
Expand Down Expand Up @@ -91,8 +91,10 @@ export class AuthProviderEntryDBImpl implements AuthProviderEntryDB {
return query.getMany();
}

protected oauthContentHash(oauth: AuthProviderEntry["oauth"]): string {
const result = createHash("sha256").update(JSON.stringify(oauth)).digest("hex");
protected oauthContentHash(entry: AuthProviderEntry): string {
const result = createHash("sha256")
.update(JSON.stringify({ oauth: entry.oauth, ownerId: entry.ownerId, status: entry.status }))
.digest("hex");
return result;
}
}
17 changes: 10 additions & 7 deletions components/server/src/auth/host-context-provider-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,19 @@ export class HostContextProviderImpl implements HostContextProvider {
const { host } = config;

const existingContext = this.dynamicHosts.get(host);
const existingConfig = existingContext && existingContext.authProvider.params;
if (existingConfig && config.id === existingConfig.id) {
if (existingConfig.host !== config.host) {
if (existingContext) {
const existingConfig = existingContext.authProvider.params;
const sameId = config.id === existingConfig.id;
const sameHost = config.host === existingConfig.host;
if (!sameHost || !sameId) {
log.warn("Ignoring host update for dynamic Auth Provider: " + host, { config, existingConfig });
continue;
}
if (existingConfig.status === config.status) {
if (!!config.oauthRevision && existingConfig.oauthRevision === config.oauthRevision) {
continue;
}
const sameOwner = config.ownerId === existingConfig.ownerId;
const sameStatus = config.status === existingConfig.status;
const hasOAuthRevisionChanged =
!!config.oauthRevision && existingConfig.oauthRevision === config.oauthRevision;
if (sameOwner && sameStatus && hasOAuthRevisionChanged) {
if (JSON.stringify(existingConfig.oauth) === JSON.stringify(config.oauth)) {
continue;
}
Expand Down
4 changes: 2 additions & 2 deletions components/server/src/auth/login-completion-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,10 @@ export class LoginCompletionHandler {
const hostCtx = this.hostContextProvider.get(hostname);
if (hostCtx) {
const { params: config } = hostCtx.authProvider;
const { id, verified, ownerId, builtin } = config;
const { id, verified, builtin } = config;
if (!builtin && !verified) {
try {
await this.authProviderService.markAsVerified({ id, ownerId });
await this.authProviderService.markAsVerified({ id, ownerId: user.id });
} catch (error) {
log.error(LogContext.from({ user }), `Failed to mark AuthProvider as verified!`, { error });
}
Expand Down

0 comments on commit e8020f4

Please sign in to comment.