From 39503a7c24e2c58c4d37175cbd0ce391a30e7e22 Mon Sep 17 00:00:00 2001 From: Sergei Zaychenko Date: Fri, 25 Aug 2023 02:32:30 -0700 Subject: [PATCH] Simplified authentication GraphQL: single entry point and serialized credentials specific for chosen login method --- resources/schema.graphql | 3 +- src/app/api/auth.api.model.ts | 11 ++++ src/app/api/auth.api.spec.ts | 41 ++++++------- src/app/api/auth.api.ts | 61 ++++++++++---------- src/app/api/gql/github-login.graphql | 29 ---------- src/app/api/gql/login.graphql | 18 ++++++ src/app/api/kamu.graphql.interface.ts | 73 +++++------------------- src/app/api/mock/auth.mock.ts | 15 ++--- src/app/auth/logged-user-service.spec.ts | 20 ++----- 9 files changed, 105 insertions(+), 166 deletions(-) create mode 100644 src/app/api/auth.api.model.ts delete mode 100644 src/app/api/gql/github-login.graphql create mode 100644 src/app/api/gql/login.graphql diff --git a/resources/schema.graphql b/resources/schema.graphql index 59b48184e..3b5d72623 100644 --- a/resources/schema.graphql +++ b/resources/schema.graphql @@ -44,8 +44,7 @@ type AttachmentsEmbedded { } type AuthMut { - passwordLogin(login: String!, password: String!): LoginResponse! - githubLogin(code: String!): LoginResponse! + login(loginMethod: String!, loginCredentialsJson: String!): LoginResponse! accountInfo(accessToken: String!): AccountInfo! } diff --git a/src/app/api/auth.api.model.ts b/src/app/api/auth.api.model.ts new file mode 100644 index 000000000..cb247e908 --- /dev/null +++ b/src/app/api/auth.api.model.ts @@ -0,0 +1,11 @@ +export const LOGIN_METHOD_PASSWORD = "password"; +export const LOGIN_METHOD_GITHUB = "oauth_github"; + +export interface PasswordLoginCredentials { + login: string; + password: string; +} + +export interface GithubLoginCredentials { + code: string; +} diff --git a/src/app/api/auth.api.spec.ts b/src/app/api/auth.api.spec.ts index 578fd769b..559d119fd 100644 --- a/src/app/api/auth.api.spec.ts +++ b/src/app/api/auth.api.spec.ts @@ -1,12 +1,7 @@ import { fakeAsync, flush, TestBed, tick } from "@angular/core/testing"; import { Apollo } from "apollo-angular"; import { AuthApi } from "./auth.api"; -import { - AccountDetailsFragment, - FetchAccountInfoDocument, - GithubLoginDocument, - PasswordLoginDocument, -} from "./kamu.graphql.interface"; +import { AccountDetailsFragment, FetchAccountInfoDocument, LoginDocument } from "./kamu.graphql.interface"; import { ApolloTestingController, ApolloTestingModule } from "apollo-angular/testing"; import { mockAccountDetails, @@ -21,6 +16,12 @@ import { } from "./mock/auth.mock"; import { AuthenticationError } from "../common/errors"; import { first } from "rxjs/operators"; +import { + GithubLoginCredentials, + LOGIN_METHOD_GITHUB, + LOGIN_METHOD_PASSWORD, + PasswordLoginCredentials, +} from "./auth.api.model"; describe("AuthApi", () => { let service: AuthApi; @@ -53,8 +54,11 @@ describe("AuthApi", () => { function loginFullyViaGithub(): void { service.fetchUserInfoAndTokenFromGithubCallackCode(TEST_GITHUB_CODE).subscribe(); - const op = controller.expectOne(GithubLoginDocument); - expect(op.operation.variables.code).toEqual(TEST_GITHUB_CODE); + const expectedCredentials: GithubLoginCredentials = { code: TEST_GITHUB_CODE }; + + const op = controller.expectOne(LoginDocument); + expect(op.operation.variables.login_method).toEqual(LOGIN_METHOD_GITHUB); + expect(op.operation.variables.login_credentials_json).toEqual(JSON.stringify(expectedCredentials)); op.flush({ data: mockGithubLoginResponse, @@ -64,9 +68,11 @@ describe("AuthApi", () => { function loginFullyViaPassword(): void { service.fetchUserInfoAndTokenFromPasswordLogin(TEST_LOGIN, TEST_PASSWORD).subscribe(); - const op = controller.expectOne(PasswordLoginDocument); - expect(op.operation.variables.login).toEqual(TEST_LOGIN); - expect(op.operation.variables.password).toEqual(TEST_PASSWORD); + const expectedCredentials: PasswordLoginCredentials = { login: TEST_LOGIN, password: TEST_PASSWORD }; + + const op = controller.expectOne(LoginDocument); + expect(op.operation.variables.login_method).toEqual(LOGIN_METHOD_PASSWORD); + expect(op.operation.variables.login_credentials_json).toEqual(JSON.stringify(expectedCredentials)); op.flush({ data: mockPasswordLoginResponse, @@ -82,7 +88,7 @@ describe("AuthApi", () => { .accessTokenObtained() .pipe(first()) .subscribe((token: string) => { - expect(token).toEqual(mockPasswordLoginResponse.auth.passwordLogin.accessToken); + expect(token).toEqual(mockPasswordLoginResponse.auth.login.accessToken); }); const accountChanged$ = service @@ -111,10 +117,7 @@ describe("AuthApi", () => { }, }); - const op = controller.expectOne(PasswordLoginDocument); - expect(op.operation.variables.login).toEqual(TEST_LOGIN); - expect(op.operation.variables.password).toEqual(TEST_PASSWORD); - + const op = controller.expectOne(LoginDocument); op.graphqlErrors([mockLogin401Error]); tick(); @@ -127,7 +130,7 @@ describe("AuthApi", () => { .accessTokenObtained() .pipe(first()) .subscribe((token: string) => { - expect(token).toEqual(mockGithubLoginResponse.auth.githubLogin.accessToken); + expect(token).toEqual(mockGithubLoginResponse.auth.login.accessToken); }); const accountChanged$ = service @@ -156,9 +159,7 @@ describe("AuthApi", () => { }, }); - const op = controller.expectOne(GithubLoginDocument); - expect(op.operation.variables.code).toEqual(TEST_GITHUB_CODE); - + const op = controller.expectOne(LoginDocument); op.graphqlErrors([mockLogin401Error]); tick(); diff --git a/src/app/api/auth.api.ts b/src/app/api/auth.api.ts index 94468e577..d51cb7408 100644 --- a/src/app/api/auth.api.ts +++ b/src/app/api/auth.api.ts @@ -5,23 +5,23 @@ import { AccountDetailsFragment, FetchAccountInfoGQL, FetchAccountInfoMutation, - GithubLoginGQL, - GithubLoginMutation, - PasswordLoginGQL, - PasswordLoginMutation, + LoginGQL, + LoginMutation, } from "./kamu.graphql.interface"; import { MutationResult } from "apollo-angular"; import { AuthenticationError } from "../common/errors"; +import { + GithubLoginCredentials, + LOGIN_METHOD_GITHUB, + LOGIN_METHOD_PASSWORD, + PasswordLoginCredentials, +} from "./auth.api.model"; @Injectable({ providedIn: "root", }) export class AuthApi { - constructor( - private githubLoginGQL: GithubLoginGQL, - private passwordLoginGQL: PasswordLoginGQL, - private fetchAccountInfoGQL: FetchAccountInfoGQL, - ) {} + constructor(private loginGQL: LoginGQL, private fetchAccountInfoGQL: FetchAccountInfoGQL) {} private accessTokenObtained$: Subject = new ReplaySubject(1); private accountChanged$: Subject = new ReplaySubject(1); @@ -35,27 +35,21 @@ export class AuthApi { } public fetchUserInfoAndTokenFromPasswordLogin(login: string, password: string): Observable { - return this.passwordLoginGQL.mutate({ login, password }).pipe( - map((result: MutationResult) => { - if (result.data) { - const data: PasswordLoginMutation = result.data; - this.accessTokenObtained$.next(data.auth.passwordLogin.accessToken); - this.accountChanged$.next(data.auth.passwordLogin.accountInfo); - } else { - throw new AuthenticationError(result.errors ?? []); - } - }), - catchError((e: Error) => throwError(() => new AuthenticationError([e]))), - ); + const passwordCredentials: PasswordLoginCredentials = { login, password }; + return this.fetchUserInfoAndTokenFromLoginMethod(LOGIN_METHOD_PASSWORD, passwordCredentials); } public fetchUserInfoAndTokenFromGithubCallackCode(code: string): Observable { - return this.githubLoginGQL.mutate({ code }).pipe( - map((result: MutationResult) => { + const githubCredentials: GithubLoginCredentials = { code }; + return this.fetchUserInfoAndTokenFromLoginMethod(LOGIN_METHOD_GITHUB, githubCredentials); + } + + public fetchUserInfoFromAccessToken(accessToken: string): Observable { + return this.fetchAccountInfoGQL.mutate({ accessToken }).pipe( + map((result: MutationResult) => { if (result.data) { - const data: GithubLoginMutation = result.data; - this.accessTokenObtained$.next(data.auth.githubLogin.accessToken); - this.accountChanged$.next(data.auth.githubLogin.accountInfo); + const data: FetchAccountInfoMutation = result.data; + this.accountChanged$.next(data.auth.accountInfo); } else { throw new AuthenticationError(result.errors ?? []); } @@ -64,12 +58,17 @@ export class AuthApi { ); } - public fetchUserInfoFromAccessToken(accessToken: string): Observable { - return this.fetchAccountInfoGQL.mutate({ accessToken }).pipe( - map((result: MutationResult) => { + private fetchUserInfoAndTokenFromLoginMethod( + loginMethod: string, + loginCredentials: TCredentials, + ): Observable { + const loginCredentialsJson: string = JSON.stringify(loginCredentials); + return this.loginGQL.mutate({ login_method: loginMethod, login_credentials_json: loginCredentialsJson }).pipe( + map((result: MutationResult) => { if (result.data) { - const data: FetchAccountInfoMutation = result.data; - this.accountChanged$.next(data.auth.accountInfo); + const data: LoginMutation = result.data; + this.accessTokenObtained$.next(data.auth.login.accessToken); + this.accountChanged$.next(data.auth.login.accountInfo); } else { throw new AuthenticationError(result.errors ?? []); } diff --git a/src/app/api/gql/github-login.graphql b/src/app/api/gql/github-login.graphql deleted file mode 100644 index 9dd6b5bec..000000000 --- a/src/app/api/gql/github-login.graphql +++ /dev/null @@ -1,29 +0,0 @@ -mutation GithubLogin($code: String!) { - auth { - githubLogin(code: $code) { - accessToken - accountInfo { - ...AccountDetails - } - } - } -} - -mutation PasswordLogin($login: String!, $password: String!) { - auth { - passwordLogin(login: $login, password: $password) { - accessToken - accountInfo { - ...AccountDetails - } - } - } -} - -mutation FetchAccountInfo($accessToken: String!) { - auth { - accountInfo(accessToken: $accessToken) { - ...AccountDetails - } - } -} diff --git a/src/app/api/gql/login.graphql b/src/app/api/gql/login.graphql new file mode 100644 index 000000000..03d92e47f --- /dev/null +++ b/src/app/api/gql/login.graphql @@ -0,0 +1,18 @@ +mutation Login($login_method: String!, $login_credentials_json: String!) { + auth { + login(loginMethod: $login_method, loginCredentialsJson: $login_credentials_json) { + accessToken + accountInfo { + ...AccountDetails + } + } + } +} + +mutation FetchAccountInfo($accessToken: String!) { + auth { + accountInfo(accessToken: $accessToken) { + ...AccountDetails + } + } +} diff --git a/src/app/api/kamu.graphql.interface.ts b/src/app/api/kamu.graphql.interface.ts index 94bad0b9e..63c26dc52 100644 --- a/src/app/api/kamu.graphql.interface.ts +++ b/src/app/api/kamu.graphql.interface.ts @@ -84,21 +84,16 @@ export type AttachmentsEmbedded = { export type AuthMut = { __typename?: "AuthMut"; accountInfo: AccountInfo; - githubLogin: LoginResponse; - passwordLogin: LoginResponse; + login: LoginResponse; }; export type AuthMutAccountInfoArgs = { accessToken: Scalars["String"]; }; -export type AuthMutGithubLoginArgs = { - code: Scalars["String"]; -}; - -export type AuthMutPasswordLoginArgs = { - login: Scalars["String"]; - password: Scalars["String"]; +export type AuthMutLoginArgs = { + loginCredentialsJson: Scalars["String"]; + loginMethod: Scalars["String"]; }; export type BlockInterval = { @@ -1747,32 +1742,16 @@ export type MetadataBlockFragment = { | ({ __typename: "SetWatermark" } & SetWatermarkEventFragment); }; -export type GithubLoginMutationVariables = Exact<{ - code: Scalars["String"]; +export type LoginMutationVariables = Exact<{ + login_method: Scalars["String"]; + login_credentials_json: Scalars["String"]; }>; -export type GithubLoginMutation = { +export type LoginMutation = { __typename?: "Mutation"; auth: { __typename?: "AuthMut"; - githubLogin: { - __typename?: "LoginResponse"; - accessToken: string; - accountInfo: { __typename?: "AccountInfo" } & AccountDetailsFragment; - }; - }; -}; - -export type PasswordLoginMutationVariables = Exact<{ - login: Scalars["String"]; - password: Scalars["String"]; -}>; - -export type PasswordLoginMutation = { - __typename?: "Mutation"; - auth: { - __typename?: "AuthMut"; - passwordLogin: { + login: { __typename?: "LoginResponse"; accessToken: string; accountInfo: { __typename?: "AccountInfo" } & AccountDetailsFragment; @@ -2799,34 +2778,10 @@ export class EnginesGQL extends Apollo.Query { - document = GithubLoginDocument; - - constructor(apollo: Apollo.Apollo) { - super(apollo); - } -} -export const PasswordLoginDocument = gql` - mutation PasswordLogin($login: String!, $password: String!) { +export const LoginDocument = gql` + mutation Login($login_method: String!, $login_credentials_json: String!) { auth { - passwordLogin(login: $login, password: $password) { + login(loginMethod: $login_method, loginCredentialsJson: $login_credentials_json) { accessToken accountInfo { ...AccountDetails @@ -2840,8 +2795,8 @@ export const PasswordLoginDocument = gql` @Injectable({ providedIn: "root", }) -export class PasswordLoginGQL extends Apollo.Mutation { - document = PasswordLoginDocument; +export class LoginGQL extends Apollo.Mutation { + document = LoginDocument; constructor(apollo: Apollo.Apollo) { super(apollo); diff --git a/src/app/api/mock/auth.mock.ts b/src/app/api/mock/auth.mock.ts index db847b3da..2230733b1 100644 --- a/src/app/api/mock/auth.mock.ts +++ b/src/app/api/mock/auth.mock.ts @@ -1,10 +1,5 @@ import { GraphQLError } from "graphql"; -import { - AccountDetailsFragment, - FetchAccountInfoMutation, - GithubLoginMutation, - PasswordLoginMutation, -} from "../kamu.graphql.interface"; +import { AccountDetailsFragment, FetchAccountInfoMutation, LoginMutation } from "../kamu.graphql.interface"; import AppValues from "src/app/common/app.values"; export const TEST_GITHUB_CODE = "12345"; @@ -26,10 +21,10 @@ export const mockUserInfoFromAccessToken: FetchAccountInfoMutation = { }, }; -export const mockGithubLoginResponse: GithubLoginMutation = { +export const mockGithubLoginResponse: LoginMutation = { auth: { __typename: "AuthMut", - githubLogin: { + login: { __typename: "LoginResponse", accessToken: TEST_ACCESS_TOKEN_GITHUB, accountInfo: mockAccountDetails, @@ -37,10 +32,10 @@ export const mockGithubLoginResponse: GithubLoginMutation = { }, }; -export const mockPasswordLoginResponse: PasswordLoginMutation = { +export const mockPasswordLoginResponse: LoginMutation = { auth: { __typename: "AuthMut", - passwordLogin: { + login: { __typename: "LoginResponse", accessToken: TEST_ACCESS_TOKEN_PASSWORD, accountInfo: mockAccountDetails, diff --git a/src/app/auth/logged-user-service.spec.ts b/src/app/auth/logged-user-service.spec.ts index 28a6be048..07c7aa851 100644 --- a/src/app/auth/logged-user-service.spec.ts +++ b/src/app/auth/logged-user-service.spec.ts @@ -14,12 +14,7 @@ import { mockPasswordLoginResponse, mockUserInfoFromAccessToken, } from "../api/mock/auth.mock"; -import { - AccountDetailsFragment, - FetchAccountInfoDocument, - GithubLoginDocument, - PasswordLoginDocument, -} from "../api/kamu.graphql.interface"; +import { AccountDetailsFragment, FetchAccountInfoDocument, LoginDocument } from "../api/kamu.graphql.interface"; import { first } from "rxjs/operators"; import { MaybeNull } from "../common/app.types"; import AppValues from "../common/app.values"; @@ -66,9 +61,7 @@ describe("LoggedUserService", () => { function loginFullyViaGithub(): void { authApi.fetchUserInfoAndTokenFromGithubCallackCode(TEST_GITHUB_CODE).subscribe(); - const op = controller.expectOne(GithubLoginDocument); - expect(op.operation.variables.code).toEqual(TEST_GITHUB_CODE); - + const op = controller.expectOne(LoginDocument); op.flush({ data: mockGithubLoginResponse, }); @@ -77,10 +70,7 @@ describe("LoggedUserService", () => { function loginFullyViaPassword(): void { authApi.fetchUserInfoAndTokenFromPasswordLogin(TEST_LOGIN, TEST_PASSWORD).subscribe(); - const op = controller.expectOne(PasswordLoginDocument); - expect(op.operation.variables.login).toEqual(TEST_LOGIN); - expect(op.operation.variables.password).toEqual(TEST_PASSWORD); - + const op = controller.expectOne(LoginDocument); op.flush({ data: mockPasswordLoginResponse, }); @@ -132,7 +122,7 @@ describe("LoggedUserService", () => { expect(localStorageSetItemSpy).toHaveBeenCalledWith( AppValues.LOCAL_STORAGE_ACCESS_TOKEN, - mockGithubLoginResponse.auth.githubLogin.accessToken, + mockGithubLoginResponse.auth.login.accessToken, ); expect(userChanges$.closed).toBeTrue(); @@ -153,7 +143,7 @@ describe("LoggedUserService", () => { expect(localStorageSetItemSpy).toHaveBeenCalledWith( AppValues.LOCAL_STORAGE_ACCESS_TOKEN, - mockPasswordLoginResponse.auth.passwordLogin.accessToken, + mockPasswordLoginResponse.auth.login.accessToken, ); expect(userChanges$.closed).toBeTrue();