From 4f3e537c3d8d583c240b7bdd510f5323d37f99be Mon Sep 17 00:00:00 2001 From: Raghunandan Date: Mon, 29 Jun 2020 10:44:53 +0200 Subject: [PATCH] Persist scopes in Github provider and add it to the auth response. Fix serialization for Set type in locaStorage. --- .../AuthSessionManager/AuthSessionStore.ts | 22 ++++++++++- plugins/auth-backend/src/lib/OAuthProvider.ts | 28 +++++++++++++ .../src/providers/github/provider.ts | 1 + .../components/ClusterList/ClusterList.tsx | 8 ++-- .../components/ClusterPage/ClusterPage.tsx | 39 ++++++++++--------- .../ProfileCatalog/ProfileCatalog.tsx | 39 ++++++++++--------- yarn.lock | 2 +- 7 files changed, 95 insertions(+), 44 deletions(-) diff --git a/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.ts b/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.ts index 3e3a40d02a..f2b8558f74 100644 --- a/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.ts +++ b/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.ts @@ -36,6 +36,8 @@ type Options = { /** * AuthSessionStore decorates another SessionManager with a functionality * to store the session in local storage. + * + * Session is serialized to JSON with special support for following types: Set. */ export class AuthSessionStore implements SessionManager { private readonly manager: SessionManager; @@ -90,7 +92,12 @@ export class AuthSessionStore implements SessionManager { try { const sessionJson = localStorage.getItem(this.storageKey); if (sessionJson) { - const session = JSON.parse(sessionJson); + const session = JSON.parse(sessionJson, (_key, value) => { + if (value?.__type === 'Set') { + return new Set(value.__value); + } + return value; + }); return session; } @@ -105,7 +112,18 @@ export class AuthSessionStore implements SessionManager { if (session === undefined) { localStorage.removeItem(this.storageKey); } else { - localStorage.setItem(this.storageKey, JSON.stringify(session)); + localStorage.setItem( + this.storageKey, + JSON.stringify(session, (_key, value) => { + if (value instanceof Set) { + return { + __type: 'Set', + __value: Array.from(value), + }; + } + return value; + }), + ); } } } diff --git a/plugins/auth-backend/src/lib/OAuthProvider.ts b/plugins/auth-backend/src/lib/OAuthProvider.ts index b7179659f9..5434b6a4ba 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.ts @@ -33,6 +33,7 @@ export type Options = { providerId: string; secure: boolean; disableRefresh?: boolean; + persistScopes?: boolean; baseUrl: string; appOrigin: string; tokenIssuer: TokenIssuer; @@ -105,6 +106,10 @@ export class OAuthProvider implements AuthProviderRouteHandlers { throw new InputError('missing scope parameter'); } + if (this.options.persistScopes) { + this.setScopesCookie(res, scope); + } + const nonce = crypto.randomBytes(16).toString('base64'); // set a nonce cookie before redirecting to oauth provider this.setNonceCookie(res, nonce); @@ -137,6 +142,14 @@ export class OAuthProvider implements AuthProviderRouteHandlers { req, ); + if (this.options.persistScopes) { + const grantedScopes = this.getScopesFromCookie( + req, + this.options.providerId, + ); + response.providerInfo.scope = grantedScopes; + } + if (!this.options.disableRefresh) { // throw error if missing refresh token if (!refreshToken) { @@ -241,6 +254,21 @@ export class OAuthProvider implements AuthProviderRouteHandlers { }); }; + private setScopesCookie = (res: express.Response, scope: string) => { + res.cookie(`${this.options.providerId}-scope`, scope, { + maxAge: TEN_MINUTES_MS, + secure: this.options.secure, + sameSite: 'none', + domain: this.domain, + path: `${this.basePath}/${this.options.providerId}/handler`, + httpOnly: true, + }); + }; + + private getScopesFromCookie = (req: express.Request, providerId: string) => { + return req.cookies[`${providerId}-scope`]; + }; + private setRefreshTokenCookie = ( res: express.Response, refreshToken: string, diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts index 01652a6d7e..ec444733ba 100644 --- a/plugins/auth-backend/src/providers/github/provider.ts +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -115,6 +115,7 @@ export function createGithubProvider( envProviders[env] = new OAuthProvider(new GithubAuthProvider(opts), { disableRefresh: true, + persistScopes: true, providerId: 'github', secure, baseUrl, diff --git a/plugins/gitops-profiles/src/components/ClusterList/ClusterList.tsx b/plugins/gitops-profiles/src/components/ClusterList/ClusterList.tsx index c4b94e2958..44578560e6 100644 --- a/plugins/gitops-profiles/src/components/ClusterList/ClusterList.tsx +++ b/plugins/gitops-profiles/src/components/ClusterList/ClusterList.tsx @@ -41,9 +41,11 @@ const ClusterList: FC<{}> = () => { const { loading, error, value } = useAsync( async () => { - const accessToken = await githubAuth.getAccessToken(); - const userInfo = await api.fetchUserInfo({ accessToken }); - setGithubUsername(userInfo.login); + const accessToken = await githubAuth.getAccessToken(['repo', 'user']); + if (!githubUsername) { + const userInfo = await api.fetchUserInfo({ accessToken }); + setGithubUsername(userInfo.login); + } return api.listClusters({ gitHubToken: accessToken, gitHubUser: githubUsername, diff --git a/plugins/gitops-profiles/src/components/ClusterPage/ClusterPage.tsx b/plugins/gitops-profiles/src/components/ClusterPage/ClusterPage.tsx index c66b0613b5..b2d62e8650 100644 --- a/plugins/gitops-profiles/src/components/ClusterPage/ClusterPage.tsx +++ b/plugins/gitops-profiles/src/components/ClusterPage/ClusterPage.tsx @@ -52,7 +52,7 @@ const ClusterPage: FC<{}> = () => { useEffect(() => { const fetchGithubUserInfo = async () => { - const accessToken = await githubAuth.getAccessToken(); + const accessToken = await githubAuth.getAccessToken(['repo', 'user']); const userInfo = await api.fetchUserInfo({ accessToken }); setGithubAccessToken(accessToken); setGithubUsername(userInfo.login); @@ -60,26 +60,27 @@ const ClusterPage: FC<{}> = () => { if (!githubAccessToken || !githubUsername) { fetchGithubUserInfo(); + } else { + if (pollingLog) { + const interval = setInterval(async () => { + const resp = await api.fetchLog({ + gitHubToken: githubAccessToken, + gitHubUser: githubUsername, + targetOrg: params.owner, + targetRepo: params.repo, + }); + + setRunStatus(resp.result); + setRunLink(resp.link); + if (resp.status === 'completed') { + setPollingLog(false); + setShowProgress(false); + } + }, 10000); + return () => clearInterval(interval); + } } - if (pollingLog) { - const interval = setInterval(async () => { - const resp = await api.fetchLog({ - gitHubToken: githubAccessToken, - gitHubUser: githubUsername, - targetOrg: params.owner, - targetRepo: params.repo, - }); - - setRunStatus(resp.result); - setRunLink(resp.link); - if (resp.status === 'completed') { - setPollingLog(false); - setShowProgress(false); - } - }, 10000); - return () => clearInterval(interval); - } return () => {}; }, [pollingLog, api, params, githubAuth, githubAccessToken, githubUsername]); diff --git a/plugins/gitops-profiles/src/components/ProfileCatalog/ProfileCatalog.tsx b/plugins/gitops-profiles/src/components/ProfileCatalog/ProfileCatalog.tsx index 619aba447c..3193bbea06 100644 --- a/plugins/gitops-profiles/src/components/ProfileCatalog/ProfileCatalog.tsx +++ b/plugins/gitops-profiles/src/components/ProfileCatalog/ProfileCatalog.tsx @@ -131,7 +131,7 @@ const ProfileCatalog: FC<{}> = () => { useEffect(() => { const fetchGithubUserInfo = async () => { - const accessToken = await githubAuth.getAccessToken(); + const accessToken = await githubAuth.getAccessToken(['repo', 'user']); const userInfo = await api.fetchUserInfo({ accessToken }); setGithubAccessToken(accessToken); setGithubUsername(userInfo.login); @@ -140,26 +140,27 @@ const ProfileCatalog: FC<{}> = () => { if (!githubAccessToken || !githubUsername) { fetchGithubUserInfo(); + } else { + if (pollingLog) { + const interval = setInterval(async () => { + const resp = await api.fetchLog({ + gitHubToken: githubAccessToken, + gitHubUser: githubUsername, + targetOrg: gitHubOrg, + targetRepo: gitHubRepo, + }); + + setRunStatus(resp.result); + setRunLink(resp.link); + if (resp.status === 'completed') { + setPollingLog(false); + setShowProgress(false); + } + }, 10000); + return () => clearInterval(interval); + } } - if (pollingLog) { - const interval = setInterval(async () => { - const resp = await api.fetchLog({ - gitHubToken: githubAccessToken, - gitHubUser: githubUsername, - targetOrg: gitHubOrg, - targetRepo: gitHubRepo, - }); - - setRunStatus(resp.result); - setRunLink(resp.link); - if (resp.status === 'completed') { - setPollingLog(false); - setShowProgress(false); - } - }, 10000); - return () => clearInterval(interval); - } return () => {}; }, [ pollingLog, diff --git a/yarn.lock b/yarn.lock index 65f48de993..9b25df19cc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -17873,7 +17873,7 @@ tiny-emitter@^2.0.0: resolved "https://registry.npmjs.org/tiny-emitter/-/tiny-emitter-2.1.0.tgz#1d1a56edfc51c43e863cbb5382a72330e3555423" integrity sha512-NB6Dk1A9xgQPMoGqC5CVXn123gWyte215ONT5Pp5a0yt4nlEoO1ZWeCwpncaekPHXO60i47ihFnZPiRPjRMq4Q== -tiny-invariant@^1.0.4, tiny-invariant@^1.0.6: +tiny-invariant@^1.0.6: version "1.1.0" resolved "https://registry.npmjs.org/tiny-invariant/-/tiny-invariant-1.1.0.tgz#634c5f8efdc27714b7f386c35e6760991d230875" integrity sha512-ytxQvrb1cPc9WBEI/HSeYYoGD0kWnGEOR8RY6KomWLBVhqz0RgTwVO9dLrGz7dC+nN9llyI7OKAgRq8Vq4ZBSw==