From ad5beec3849a189e639749f6b6a92a61917044ae Mon Sep 17 00:00:00 2001 From: Baghirov Feyruz Date: Mon, 9 Dec 2024 14:42:33 +0100 Subject: [PATCH] fixed unnescessary passing of state and code_challenge to client --- .../codechecker_server/api/authentication.py | 25 +++++++++++++------ web/server/vue-cli/src/views/Login.vue | 7 ------ web/server/vue-cli/src/views/OAuthLogin.vue | 22 ++++------------ web/tests/libtest/env.py | 9 ++++--- 4 files changed, 28 insertions(+), 35 deletions(-) diff --git a/web/server/codechecker_server/api/authentication.py b/web/server/codechecker_server/api/authentication.py index 3992a24c06..365db06a62 100644 --- a/web/server/codechecker_server/api/authentication.py +++ b/web/server/codechecker_server/api/authentication.py @@ -237,14 +237,11 @@ def createLink(self, provider): ) # Create authorization URL - nonce = generate_token() url, state = session.create_authorization_url( authorization_uri, - nonce=nonce, state=stored_state, code_verifier=pkce_verifier ) - # Save the state and nonce to the database oauth_data_id = self.insertDataOauth(state=state, code_verifier=pkce_verifier, @@ -254,7 +251,7 @@ def createLink(self, provider): codechecker_api_shared.ttypes.ErrorCode.AUTH_DENIED, "OAuth data insertion failed.") - LOG.debug("State inserted successfully with ID %s", oauth_data_id) + LOG.info("Data inserted successfully with ID %s", oauth_data_id) return url + "&oauth_data_id=" + str(oauth_data_id) @timeit @@ -289,8 +286,6 @@ def performLogin(self, auth_method, auth_string): parsed_query = parse_qs(url_new.query) code = parsed_query.get("code")[0] state = parsed_query.get("state")[0] - cc = parsed_query.get("code_challenge")[0] - cc_method = parsed_query.get("code_challenge_method")[0] oauth_data_id = parsed_query.get("oauth_data_id")[0] # for code verifier from created link and original code_verifier_db = None @@ -325,6 +320,7 @@ def performLogin(self, auth_method, auth_string): client_id = oauth_config["oauth_client_id"] client_secret = oauth_config["oauth_client_secret"] scope = oauth_config["oauth_scope"] + authorization_uri = oauth_config["oauth_authorization_uri"] token_url = oauth_config["oauth_token_uri"] user_info_url = oauth_config["oauth_user_info_uri"] callback_url = oauth_config["oauth_callback_url"] @@ -346,6 +342,19 @@ def performLogin(self, auth_method, auth_string): codechecker_api_shared.ttypes.ErrorCode.AUTH_DENIED, "OAuth2Session creation failed.") + # recreating the url for pkce purpose + # no way to get the original session so recreate it + # no returning url or state + url_recreated = session.create_authorization_url( + authorization_uri, + state=state_db, + code_verifier=code_verifier_db + )[0] + + hashed_challenge = urlparse(url_recreated) + parsed_query_r = parse_qs(hashed_challenge.query) + cc = parsed_query_r.get("code_challenge")[0] + # FIXME: This is a workaround for the Microsoft OAuth2 provider # which doesn't correctly fetch the code from url. # the workaround is to construct the url manually @@ -353,12 +362,12 @@ def performLogin(self, auth_method, auth_string): url = url_new.scheme + "://" + url_new.netloc + url_new.path + \ "?code=" + code + "&state=" + state + \ "&code_challenge=" + cc + \ - "&code_challenge_method=" + cc_method + "&code_challenge_method=S256" LOG.info("URL has been constructed successfully") token = None try: - # code_verifier_db is not supported for github provider + # code_verifier_db or PKCE is't supported by github # if it will be fixed the code should adjust automatically token = session.fetch_token( url=token_url, diff --git a/web/server/vue-cli/src/views/Login.vue b/web/server/vue-cli/src/views/Login.vue index 6d8ddd0840..9e0bb56647 100644 --- a/web/server/vue-cli/src/views/Login.vue +++ b/web/server/vue-cli/src/views/Login.vue @@ -211,7 +211,6 @@ export default { oauth(provider) { new Promise(resolve => { localStorage.setItem("oauth_provider", provider); - // console output the provider for debugging authService.getClient().createLink(provider, handleThriftError(url => { resolve(url); @@ -221,14 +220,8 @@ export default { this.success = false; this.error = false; const params = new URLSearchParams(url); - localStorage.setItem("oauth_state", - params.get("state")); localStorage.setItem("oauth_data_id", params.get("oauth_data_id")); - localStorage.setItem("code_challenge", - params.get("code_challenge")); - localStorage.setItem("method", - params.get("code_challenge_method")); window.location.href = url; } else { diff --git a/web/server/vue-cli/src/views/OAuthLogin.vue b/web/server/vue-cli/src/views/OAuthLogin.vue index 50e7d0b57a..75a7452130 100644 --- a/web/server/vue-cli/src/views/OAuthLogin.vue +++ b/web/server/vue-cli/src/views/OAuthLogin.vue @@ -55,26 +55,14 @@ export default { methods: { detectCallback() { - const url = this.$route.query; + const params = this.$route.query; + const url = window.location.href; const provider = this.$route.params.provider; - const state = localStorage.getItem("oauth_state"); - const code_challenge = localStorage.getItem("code_challenge"); - const method = localStorage.getItem("method"); - if (url.code != null && url.state != null) { - if (url.state != state) { - this.errorMsg = "Invalid state!"; - this.error = true; - this.callback = true; - return; - } + if (params.code != null && params.state != null) { const oauth_data_id = localStorage.getItem("oauth_data_id"); - const baseUrl = window.location.href.replace("#", ""); - const baseUrlOAuthId = `${baseUrl}&oauth_data_id=${oauth_data_id}`; - const baseMethod = `${baseUrlOAuthId}&code_challenge_method=${method}`; - const fullUrl = `${baseMethod}&code_challenge=${code_challenge}`; - + const fullUrl = `${url}&oauth_data_id=${oauth_data_id}`; this.$store .dispatch(LOGIN, { type: "oauth", @@ -93,7 +81,7 @@ export default { this.$router.replace({ name: "login" }); }); } - } + }, } }; diff --git a/web/tests/libtest/env.py b/web/tests/libtest/env.py index 02e59b985b..b26de5339f 100644 --- a/web/tests/libtest/env.py +++ b/web/tests/libtest/env.py @@ -366,7 +366,8 @@ def enable_auth(workspace): "oauth_client_id": "1", "oauth_client_secret": "1", "oauth_authorization_uri": "http://localhost:3000/login", - "oauth_callback_url": "http://localhost:8080/login/OAuthLogin/github", + "oauth_callback_url": "http://localhost:8080/login/" + + "OAuthLogin/github", "oauth_token_uri": "http://localhost:3000/token", "oauth_user_info_uri": "http://localhost:3000/get_user", "oauth_scope": "openid email profile", @@ -382,7 +383,8 @@ def enable_auth(workspace): "oauth_client_id": "2", "oauth_client_secret": "2", "oauth_authorization_uri": "http://localhost:3000/login", - "oauth_callback_url": "http://localhost:8080/login/OAuthLogin/google", + "oauth_callback_url": "http://localhost:8080/login/" + + "OAuthLogin/google", "oauth_token_uri": "http://localhost:3000/token", "oauth_user_info_uri": "http://localhost:3000/get_user", "oauth_scope": "openid email profile", @@ -398,7 +400,8 @@ def enable_auth(workspace): "oauth_client_id": "3", "oauth_client_secret": "3", "oauth_authorization_uri": "http://localhost:3000/login", - "oauth_callback_url": "http://localhost:8080/login/OAuthLogin/dummy", + "oauth_callback_url": "http://localhost:8080/login/" + + "OAuthLogin/dummy", "oauth_token_uri": "http://localhost:3000/token", "oauth_user_info_uri": "http://localhost:3000/get_user", "oauth_scope": "openid email profile",