From e24d0ce6055957acab609426453dc29231d557a2 Mon Sep 17 00:00:00 2001 From: Akash K <57758277+amk-dev@users.noreply.github.com> Date: Tue, 14 Nov 2023 00:12:04 +0530 Subject: [PATCH] fix: oauth 2.0 authentication type is breaking (#3531) --- packages/hoppscotch-common/package.json | 2 +- .../components/http/OAuth2Authorization.vue | 7 +- .../src/helpers/{oauth.js => oauth.ts} | 182 ++++++++++++------ .../hoppscotch-common/src/pages/index.vue | 26 +-- .../hoppscotch-common/src/pages/oauth.vue | 43 +++++ pnpm-lock.yaml | 22 +-- 6 files changed, 187 insertions(+), 95 deletions(-) rename packages/hoppscotch-common/src/helpers/{oauth.js => oauth.ts} (53%) create mode 100644 packages/hoppscotch-common/src/pages/oauth.vue diff --git a/packages/hoppscotch-common/package.json b/packages/hoppscotch-common/package.json index 00907506a..eb3d22c16 100644 --- a/packages/hoppscotch-common/package.json +++ b/packages/hoppscotch-common/package.json @@ -102,7 +102,7 @@ "workbox-window": "^7.0.0", "xml-formatter": "^3.5.0", "yargs-parser": "^21.1.1", - "zod": "^3.22.2" + "zod": "^3.22.4" }, "devDependencies": { "@esbuild-plugins/node-globals-polyfill": "^0.2.3", diff --git a/packages/hoppscotch-common/src/components/http/OAuth2Authorization.vue b/packages/hoppscotch-common/src/components/http/OAuth2Authorization.vue index c95979cfd..b288f6afb 100644 --- a/packages/hoppscotch-common/src/components/http/OAuth2Authorization.vue +++ b/packages/hoppscotch-common/src/components/http/OAuth2Authorization.vue @@ -43,6 +43,7 @@ import { useI18n } from "@composables/i18n" import { useToast } from "@composables/toast" import { tokenRequest } from "~/helpers/oauth" import { getCombinedEnvVariables } from "~/helpers/preRequest" +import * as E from "fp-ts/Either" const t = useI18n() const toast = useToast() @@ -98,7 +99,11 @@ const handleAccessTokenRequest = async () => { clientSecret: parseTemplateString(clientSecret.value, envVars), scope: parseTemplateString(scope.value, envVars), } - await tokenRequest(tokenReqParams) + const res = await tokenRequest(tokenReqParams) + + if (res && E.isLeft(res)) { + toast.error(res.left) + } } catch (e) { toast.error(`${e}`) } diff --git a/packages/hoppscotch-common/src/helpers/oauth.js b/packages/hoppscotch-common/src/helpers/oauth.ts similarity index 53% rename from packages/hoppscotch-common/src/helpers/oauth.js rename to packages/hoppscotch-common/src/helpers/oauth.ts index f686bd97e..6fd3b470c 100644 --- a/packages/hoppscotch-common/src/helpers/oauth.js +++ b/packages/hoppscotch-common/src/helpers/oauth.ts @@ -4,7 +4,10 @@ import { removeLocalConfig, } from "~/newstore/localpersistence" -const redirectUri = `${window.location.origin}/` +import * as E from "fp-ts/Either" +import { z } from "zod" + +const redirectUri = `${window.location.origin}/oauth` // GENERAL HELPER FUNCTIONS @@ -16,7 +19,7 @@ const redirectUri = `${window.location.origin}/` * @returns {Object} */ -const sendPostRequest = async (url, params) => { +const sendPostRequest = async (url: string, params: Record) => { const body = Object.keys(params) .map((key) => `${key}=${params[key]}`) .join("&") @@ -30,9 +33,9 @@ const sendPostRequest = async (url, params) => { try { const response = await fetch(url, options) const data = await response.json() - return data + return E.right(data) } catch (e) { - console.error(e) + return E.left("AUTH_TOKEN_REQUEST_FAILED") } } @@ -43,7 +46,7 @@ const sendPostRequest = async (url, params) => { * @returns {Object} */ -const parseQueryString = (searchQuery) => { +const parseQueryString = (searchQuery: string): Record => { if (searchQuery === "") { return {} } @@ -61,7 +64,7 @@ const parseQueryString = (searchQuery) => { * @returns {Object} */ -const getTokenConfiguration = async (endpoint) => { +const getTokenConfiguration = async (endpoint: string) => { const options = { method: "GET", headers: { @@ -71,9 +74,9 @@ const getTokenConfiguration = async (endpoint) => { try { const response = await fetch(endpoint, options) const config = await response.json() - return config + return E.right(config) } catch (e) { - console.error(e) + return E.left("OIDC_DISCOVERY_FAILED") } } @@ -97,7 +100,7 @@ const generateRandomString = () => { * @returns {Promise} */ -const sha256 = (plain) => { +const sha256 = (plain: string) => { const encoder = new TextEncoder() const data = encoder.encode(plain) return window.crypto.subtle.digest("SHA-256", data) @@ -111,15 +114,18 @@ const sha256 = (plain) => { */ const base64urlencode = ( - str // Converts the ArrayBuffer to string using Uint8 array to convert to what btoa accepts. -) => + str: ArrayBuffer // Converts the ArrayBuffer to string using Uint8 array to convert to what btoa accepts. +) => { + const hashArray = Array.from(new Uint8Array(str)) + // btoa accepts chars only within ascii 0-255 and base64 encodes them. // Then convert the base64 encoded to base64url encoded // (replace + with -, replace / with _, trim trailing =) - btoa(String.fromCharCode.apply(null, new Uint8Array(str))) + return btoa(String.fromCharCode.apply(null, hashArray)) .replace(/\+/g, "-") .replace(/\//g, "_") .replace(/=+$/, "") +} /** * Return the base64-urlencoded sha256 hash for the PKCE challenge @@ -128,13 +134,23 @@ const base64urlencode = ( * @returns {String} */ -const pkceChallengeFromVerifier = async (v) => { +const pkceChallengeFromVerifier = async (v: string) => { const hashed = await sha256(v) return base64urlencode(hashed) } // OAUTH REQUEST +type TokenRequestParams = { + oidcDiscoveryUrl: string + grantType: string + authUrl: string + accessTokenUrl: string + clientId: string + clientSecret: string + scope: string +} + /** * Initiates PKCE Auth Code flow when requested * @@ -150,16 +166,28 @@ const tokenRequest = async ({ clientId, clientSecret, scope, -}) => { +}: TokenRequestParams) => { // Check oauth configuration if (oidcDiscoveryUrl !== "") { - // eslint-disable-next-line camelcase - const { authorization_endpoint, token_endpoint } = - await getTokenConfiguration(oidcDiscoveryUrl) - // eslint-disable-next-line camelcase - authUrl = authorization_endpoint - // eslint-disable-next-line camelcase - accessTokenUrl = token_endpoint + const res = await getTokenConfiguration(oidcDiscoveryUrl) + + const OIDCConfigurationSchema = z.object({ + authorization_endpoint: z.string(), + token_endpoint: z.string(), + }) + + if (E.isLeft(res)) { + return E.left("OIDC_DISCOVERY_FAILED" as const) + } + + const parsedOIDCConfiguration = OIDCConfigurationSchema.safeParse(res.right) + + if (!parsedOIDCConfiguration.success) { + return E.left("OIDC_DISCOVERY_FAILED" as const) + } + + authUrl = parsedOIDCConfiguration.data.authorization_endpoint + accessTokenUrl = parsedOIDCConfiguration.data.token_endpoint } // Store oauth information setLocalConfig("tokenEndpoint", accessTokenUrl) @@ -190,7 +218,7 @@ const tokenRequest = async ({ )}&code_challenge_method=S256` // Redirect to the authorization server - window.location = buildUrl() + window.location.assign(buildUrl()) } // OAUTH REDIRECT HANDLING @@ -202,44 +230,84 @@ const tokenRequest = async ({ * @returns {Promise} */ -const oauthRedirect = () => { - let tokenResponse = "" - const q = parseQueryString(window.location.search.substring(1)) +const handleOAuthRedirect = async () => { + const queryParams = parseQueryString(window.location.search.substring(1)) + // Check if the server returned an error string - if (q.error) { - alert(`Error returned from authorization server: ${q.error}`) + if (queryParams.error) { + return E.left("AUTH_SERVER_RETURNED_ERROR" as const) } + + if (!queryParams.code) { + return E.left("NO_AUTH_CODE" as const) + } + // If the server returned an authorization code, attempt to exchange it for an access token - if (q.code) { - // Verify state matches what we set at the beginning - if (getLocalConfig("pkce_state") !== q.state) { - alert("Invalid state") - Promise.reject(tokenResponse) - } else { - try { - // Exchange the authorization code for an access token - tokenResponse = sendPostRequest(getLocalConfig("tokenEndpoint"), { - grant_type: "authorization_code", - code: q.code, - client_id: getLocalConfig("client_id"), - client_secret: getLocalConfig("client_secret"), - redirect_uri: redirectUri, - code_verifier: getLocalConfig("pkce_codeVerifier"), - }) - } catch (e) { - console.error(e) - return Promise.reject(tokenResponse) - } - } - // Clean these up since we don't need them anymore - removeLocalConfig("pkce_state") - removeLocalConfig("pkce_codeVerifier") - removeLocalConfig("tokenEndpoint") - removeLocalConfig("client_id") - removeLocalConfig("client_secret") - return tokenResponse + // Verify state matches what we set at the beginning + if (getLocalConfig("pkce_state") !== queryParams.state) { + return E.left("INVALID_STATE" as const) } - return Promise.reject(tokenResponse) + + const tokenEndpoint = getLocalConfig("tokenEndpoint") + const clientID = getLocalConfig("client_id") + const clientSecret = getLocalConfig("client_secret") + const codeVerifier = getLocalConfig("pkce_codeVerifier") + + if (!tokenEndpoint) { + return E.left("NO_TOKEN_ENDPOINT" as const) + } + + if (!clientID) { + return E.left("NO_CLIENT_ID" as const) + } + + if (!clientSecret) { + return E.left("NO_CLIENT_SECRET" as const) + } + + if (!codeVerifier) { + return E.left("NO_CODE_VERIFIER" as const) + } + + // Exchange the authorization code for an access token + const tokenResponse: E.Either = await sendPostRequest( + tokenEndpoint, + { + grant_type: "authorization_code", + code: queryParams.code, + client_id: clientID, + client_secret: clientSecret, + redirect_uri: redirectUri, + code_verifier: codeVerifier, + } + ) + + // Clean these up since we don't need them anymore + clearPKCEState() + + if (E.isLeft(tokenResponse)) { + return E.left("AUTH_TOKEN_REQUEST_FAILED" as const) + } + + const withAccessTokenSchema = z.object({ + access_token: z.string(), + }) + + const parsedTokenResponse = withAccessTokenSchema.safeParse( + tokenResponse.right + ) + + return parsedTokenResponse.success + ? E.right(parsedTokenResponse.data) + : E.left("AUTH_TOKEN_REQUEST_INVALID_RESPONSE" as const) } -export { tokenRequest, oauthRedirect } +const clearPKCEState = () => { + removeLocalConfig("pkce_state") + removeLocalConfig("pkce_codeVerifier") + removeLocalConfig("tokenEndpoint") + removeLocalConfig("client_id") + removeLocalConfig("client_secret") +} + +export { tokenRequest, handleOAuthRedirect } diff --git a/packages/hoppscotch-common/src/pages/index.vue b/packages/hoppscotch-common/src/pages/index.vue index f7e88a7f5..c0a506347 100644 --- a/packages/hoppscotch-common/src/pages/index.vue +++ b/packages/hoppscotch-common/src/pages/index.vue @@ -94,7 +94,7 @@ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 078919c59..0d859b7d1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -611,8 +611,8 @@ importers: specifier: ^21.1.1 version: 21.1.1 zod: - specifier: ^3.22.2 - version: 3.22.2 + specifier: ^3.22.4 + version: 3.22.4 devDependencies: '@esbuild-plugins/node-globals-polyfill': specifier: ^0.2.3 @@ -5531,9 +5531,9 @@ packages: '@parcel/watcher': optional: true dependencies: - '@babel/generator': 7.22.10 - '@babel/template': 7.22.5 - '@babel/types': 7.22.10 + '@babel/generator': 7.23.0 + '@babel/template': 7.22.15 + '@babel/types': 7.23.0 '@graphql-codegen/core': 4.0.0(graphql@16.8.0) '@graphql-codegen/plugin-helpers': 5.0.1(graphql@16.8.0) '@graphql-tools/apollo-engine-loader': 8.0.0(graphql@16.8.0) @@ -5545,7 +5545,7 @@ packages: '@graphql-tools/load': 8.0.0(graphql@16.8.0) '@graphql-tools/prisma-loader': 8.0.1(@types/node@17.0.27)(graphql@16.8.0) '@graphql-tools/url-loader': 8.0.0(@types/node@17.0.27)(graphql@16.8.0) - '@graphql-tools/utils': 10.0.5(graphql@16.8.0) + '@graphql-tools/utils': 10.0.6(graphql@16.8.0) '@whatwg-node/fetch': 0.8.8 chalk: 4.1.2 cosmiconfig: 8.2.0 @@ -7390,7 +7390,7 @@ packages: graphql: ^14.0.0 || ^15.0.0 || ^16.0.0 || ^17.0.0 dependencies: '@graphql-tools/url-loader': 8.0.0(@types/node@17.0.27)(graphql@16.8.0) - '@graphql-tools/utils': 10.0.5(graphql@16.8.0) + '@graphql-tools/utils': 10.0.6(graphql@16.8.0) '@types/js-yaml': 4.0.5 '@types/json-stable-stringify': 1.0.34 '@whatwg-node/fetch': 0.9.9 @@ -7672,10 +7672,10 @@ packages: '@types/ws': 8.5.5 '@whatwg-node/fetch': 0.8.8 graphql: 16.6.0 - isomorphic-ws: 5.0.0(ws@8.13.0) + isomorphic-ws: 5.0.0(ws@8.14.2) tslib: 2.6.2 value-or-promise: 1.0.12 - ws: 8.13.0 + ws: 8.14.2 transitivePeerDependencies: - '@types/node' - bufferutil @@ -23744,7 +23744,7 @@ packages: graphql: 16.8.1 iterall: 1.3.0 symbol-observable: 1.2.0 - ws: 7.4.6 + ws: 7.5.9 transitivePeerDependencies: - bufferutil - utf-8-validate @@ -27760,7 +27760,6 @@ packages: optional: true utf-8-validate: optional: true - dev: true /ws@8.12.1: resolution: {integrity: sha512-1qo+M9Ba+xNhPB+YTWUlK6M17brTut5EXbcBaMRN5pH5dFrXz7lzz1ChFSUq3bOUl8yEvSenhHmYUNJxFzdJew==} @@ -28069,6 +28068,7 @@ packages: /zod@3.22.2: resolution: {integrity: sha512-wvWkphh5WQsJbVk1tbx1l1Ly4yg+XecD+Mq280uBGt9wa5BKSWf4Mhp6GmrkPixhMxmabYY7RbzlwVP32pbGCg==} + dev: true /zod@3.22.4: resolution: {integrity: sha512-iC+8Io04lddc+mVqQ9AZ7OQ2MrUKGN+oIQyq1vemgt46jwCwLfhq7/pwnBnNXXXZb8VTVLKwp9EDkx+ryxIWmg==}