Skip to content

Commit 46b5f96

Browse files
committed
fixup: circular dependency
1 parent 6f8191f commit 46b5f96

File tree

2 files changed

+15
-17
lines changed

2 files changed

+15
-17
lines changed

components/server/src/auth/auth-provider-service.ts

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,13 @@ import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
1818
import fetch from "node-fetch";
1919
import { Authorizer } from "../authorization/authorizer";
2020
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
21-
import { HostContextProvider } from "./host-context-provider";
2221

2322
@injectable()
2423
export class AuthProviderService {
2524
constructor(
2625
@inject(AuthProviderEntryDB) private readonly authProviderDB: AuthProviderEntryDB,
2726
@inject(TeamDB) private readonly teamDB: TeamDB,
2827
@inject(Config) protected readonly config: Config,
29-
@inject(HostContextProvider) private readonly hostContexts: HostContextProvider,
3028
@inject(Authorizer) private readonly auth: Authorizer,
3129
) {}
3230

@@ -124,22 +122,25 @@ export class AuthProviderService {
124122
}
125123

126124
// checking for already existing runtime providers
127-
const hostContext = this.hostContexts.get(host);
128-
if (hostContext) {
129-
const builtInExists = hostContext.authProvider.params.ownerId === undefined;
130-
log.info(`Attempt to override an existing provider.`, { entry, builtInExists });
125+
const isBuiltInProvider = this.isBuiltInProvider(host);
126+
if (isBuiltInProvider) {
127+
log.info(`Attempt to override an existing provider.`, { entry });
131128
throw new ApplicationError(ErrorCodes.CONFLICT, `Attempt to override an existing provider.`);
132129
}
133-
134130
const existing = await this.authProviderDB.findByHost(entry.host);
135131
if (existing) {
136-
log.info(`Attempt to override an existing provider.`, { entry });
132+
log.info(`Provider for this host already exists.`, { entry });
137133
throw new ApplicationError(ErrorCodes.CONFLICT, `Provider for this host already exists.`);
138134
}
135+
139136
const authProvider = this.initializeNewProvider(entry);
140137
return await this.authProviderDB.storeAuthProvider(authProvider as AuthProviderEntry, true);
141138
}
142139

140+
private isBuiltInProvider(host: string) {
141+
return this.config.authProviderConfigs.some((config) => config.host.toLowerCase() === host.toLocaleLowerCase());
142+
}
143+
143144
async updateAuthProviderOfUser(userId: string, entry: AuthProviderEntry.UpdateEntry): Promise<AuthProviderEntry> {
144145
await this.auth.checkPermissionOnUser(userId, "write_info", userId);
145146

@@ -181,17 +182,16 @@ export class AuthProviderService {
181182
throw new ApplicationError(ErrorCodes.BAD_REQUEST, `Host could not be reached.`);
182183
}
183184

184-
const hostContext = this.hostContexts.get(host);
185-
if (hostContext) {
186-
const builtInExists = hostContext.authProvider.params.ownerId === undefined;
187-
log.info(`Attempt to override an existing provider.`, { newEntry, builtInExists });
185+
const isBuiltInProvider = this.isBuiltInProvider(host);
186+
if (isBuiltInProvider) {
187+
log.info(`Attempt to override an existing provider.`, { newEntry });
188188
throw new ApplicationError(ErrorCodes.CONFLICT, `Attempt to override an existing provider.`);
189189
}
190190

191191
const orgProviders = await this.authProviderDB.findByOrgId(newEntry.organizationId);
192192
const existing = orgProviders.find((p) => p.host === host);
193193
if (existing) {
194-
log.info(`Attempt to override an existing provider.`, { newEntry });
194+
log.info(`Provider for this host already exists.`, { newEntry });
195195
throw new ApplicationError(ErrorCodes.CONFLICT, `Provider for this host already exists.`);
196196
}
197197

@@ -231,7 +231,7 @@ export class AuthProviderService {
231231
return await this.authProviderDB.storeAuthProvider(authProvider as AuthProviderEntry, true);
232232
}
233233

234-
protected initializeNewProvider(newEntry: AuthProviderEntry.NewEntry): AuthProviderEntry {
234+
private initializeNewProvider(newEntry: AuthProviderEntry.NewEntry): AuthProviderEntry {
235235
const { host, type, clientId, clientSecret } = newEntry;
236236
let urls;
237237
switch (type) {
@@ -317,7 +317,7 @@ export class AuthProviderService {
317317
}
318318
}
319319

320-
protected callbackUrl = () => {
320+
private callbackUrl = () => {
321321
const pathname = `/auth/callback`;
322322
return this.config.hostUrl.with({ pathname }).toString();
323323
};

components/server/src/auth/authenticator.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import { TokenProvider } from "../user/token-provider";
1616
import { UserAuthentication } from "../user/user-authentication";
1717
import { UserService } from "../user/user-service";
1818
import { AuthFlow, AuthProvider } from "./auth-provider";
19-
import { AuthProviderService } from "./auth-provider-service";
2019
import { HostContextProvider } from "./host-context-provider";
2120
import { SignInJWT } from "./jwt";
2221

@@ -29,7 +28,6 @@ export class Authenticator {
2928
@inject(TeamDB) protected teamDb: TeamDB;
3029
@inject(HostContextProvider) protected hostContextProvider: HostContextProvider;
3130
@inject(TokenProvider) protected readonly tokenProvider: TokenProvider;
32-
@inject(AuthProviderService) protected readonly authProviderService: AuthProviderService;
3331
@inject(UserAuthentication) protected readonly userAuthentication: UserAuthentication;
3432
@inject(SignInJWT) protected readonly signInJWT: SignInJWT;
3533

0 commit comments

Comments
 (0)