1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-11-25 12:15:18 +01:00

[PM-8979] Check that user is authed before getting user config (#10031)

* Check that user is authed before getting user config

* Accept PR Suggestion

Co-authored-by: Andreas Coroiu <acoroiu@bitwarden.com>

* Use Strict Equal

---------

Co-authored-by: Andreas Coroiu <acoroiu@bitwarden.com>
This commit is contained in:
Justin Baur 2024-07-15 10:41:10 -04:00 committed by GitHub
parent 5a46c7d5cc
commit 5fcf4bbd10
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 56 additions and 6 deletions

View File

@ -716,6 +716,7 @@ export default class MainBackground {
this.environmentService, this.environmentService,
this.logService, this.logService,
this.stateProvider, this.stateProvider,
this.authService,
); );
this.cipherService = new CipherService( this.cipherService = new CipherService(

View File

@ -586,6 +586,7 @@ export class ServiceContainer {
this.environmentService, this.environmentService,
this.logService, this.logService,
this.stateProvider, this.stateProvider,
this.authService,
); );
this.cipherService = new CipherService( this.cipherService = new CipherService(

View File

@ -954,7 +954,13 @@ const safeProviders: SafeProvider[] = [
safeProvider({ safeProvider({
provide: DefaultConfigService, provide: DefaultConfigService,
useClass: DefaultConfigService, useClass: DefaultConfigService,
deps: [ConfigApiServiceAbstraction, EnvironmentService, LogService, StateProvider], deps: [
ConfigApiServiceAbstraction,
EnvironmentService,
LogService,
StateProvider,
AuthServiceAbstraction,
],
}), }),
safeProvider({ safeProvider({
provide: ConfigService, provide: ConfigService,

View File

@ -14,6 +14,8 @@ import {
mockAccountServiceWith, mockAccountServiceWith,
} from "../../../../spec"; } from "../../../../spec";
import { subscribeTo } from "../../../../spec/observable-tracker"; import { subscribeTo } from "../../../../spec/observable-tracker";
import { AuthService } from "../../../auth/abstractions/auth.service";
import { AuthenticationStatus } from "../../../auth/enums/authentication-status";
import { UserId } from "../../../types/guid"; import { UserId } from "../../../types/guid";
import { ConfigApiServiceAbstraction } from "../../abstractions/config/config-api.service.abstraction"; import { ConfigApiServiceAbstraction } from "../../abstractions/config/config-api.service.abstraction";
import { ServerConfig } from "../../abstractions/config/server-config"; import { ServerConfig } from "../../abstractions/config/server-config";
@ -39,6 +41,9 @@ describe("ConfigService", () => {
const configApiService = mock<ConfigApiServiceAbstraction>(); const configApiService = mock<ConfigApiServiceAbstraction>();
const environmentService = mock<EnvironmentService>(); const environmentService = mock<EnvironmentService>();
const logService = mock<LogService>(); const logService = mock<LogService>();
const authService = mock<AuthService>({
authStatusFor$: (userId) => of(AuthenticationStatus.Unlocked),
});
let stateProvider: FakeStateProvider; let stateProvider: FakeStateProvider;
let globalState: FakeGlobalState<Record<ApiUrl, ServerConfig>>; let globalState: FakeGlobalState<Record<ApiUrl, ServerConfig>>;
let userState: FakeSingleUserState<ServerConfig>; let userState: FakeSingleUserState<ServerConfig>;
@ -71,6 +76,7 @@ describe("ConfigService", () => {
environmentService, environmentService,
logService, logService,
stateProvider, stateProvider,
authService,
); );
}); });
@ -188,6 +194,30 @@ describe("ConfigService", () => {
}); });
}); });
it("gets global config when there is an locked active user", async () => {
await accountService.switchAccount(userId);
environmentService.environment$ = of(environmentFactory(activeApiUrl));
globalState.stateSubject.next({
[activeApiUrl]: serverConfigFactory(activeApiUrl + "global"),
});
userState.nextState(serverConfigFactory(userId));
const sut = new DefaultConfigService(
configApiService,
environmentService,
logService,
stateProvider,
mock<AuthService>({
authStatusFor$: () => of(AuthenticationStatus.Locked),
}),
);
const config = await firstValueFrom(sut.serverConfig$);
expect(config.gitHash).toEqual(activeApiUrl + "global");
});
describe("environment change", () => { describe("environment change", () => {
let sut: DefaultConfigService; let sut: DefaultConfigService;
let environmentSubject: Subject<Environment>; let environmentSubject: Subject<Environment>;
@ -205,6 +235,7 @@ describe("ConfigService", () => {
environmentService, environmentService,
logService, logService,
stateProvider, stateProvider,
authService,
); );
}); });

View File

@ -13,6 +13,8 @@ import {
} from "rxjs"; } from "rxjs";
import { SemVer } from "semver"; import { SemVer } from "semver";
import { AuthService } from "../../../auth/abstractions/auth.service";
import { AuthenticationStatus } from "../../../auth/enums/authentication-status";
import { import {
DefaultFeatureFlagValue, DefaultFeatureFlagValue,
FeatureFlag, FeatureFlag,
@ -60,16 +62,25 @@ export class DefaultConfigService implements ConfigService {
private environmentService: EnvironmentService, private environmentService: EnvironmentService,
private logService: LogService, private logService: LogService,
private stateProvider: StateProvider, private stateProvider: StateProvider,
private authService: AuthService,
) { ) {
const apiUrl$ = this.environmentService.environment$.pipe( const apiUrl$ = this.environmentService.environment$.pipe(
map((environment) => environment.getApiUrl()), map((environment) => environment.getApiUrl()),
); );
const userId$ = this.stateProvider.activeUserId$;
const authStatus$ = userId$.pipe(
switchMap((userId) => (userId == null ? of(null) : this.authService.authStatusFor$(userId))),
);
this.serverConfig$ = combineLatest([this.stateProvider.activeUserId$, apiUrl$]).pipe( this.serverConfig$ = combineLatest([userId$, apiUrl$, authStatus$]).pipe(
switchMap(([userId, apiUrl]) => { switchMap(([userId, apiUrl, authStatus]) => {
const config$ = if (userId == null || authStatus !== AuthenticationStatus.Unlocked) {
userId == null ? this.globalConfigFor$(apiUrl) : this.userConfigFor$(userId); return this.globalConfigFor$(apiUrl).pipe(
return config$.pipe(map((config) => [config, userId, apiUrl] as const)); map((config) => [config, null, apiUrl] as const),
);
}
return this.userConfigFor$(userId).pipe(map((config) => [config, userId, apiUrl] as const));
}), }),
tap(async (rec) => { tap(async (rec) => {
const [existingConfig, userId, apiUrl] = rec; const [existingConfig, userId, apiUrl] = rec;