removed oauth_data_id, was prone to attack, now uses state for queryi…

…ng data
Ericsson · Dec 9, 2024 · e3d6d7a · e3d6d7a
1 parent ad5beec
commit e3d6d7a
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 43 deletions.
diff --git a/web/server/codechecker_server/api/authentication.py b/web/server/codechecker_server/api/authentication.py
@@ -180,25 +180,11 @@ def insertDataOauth(self, state, code_verifier, provider):
                                          provider=provider)
                 session.add(new_state)
                 session.commit()
-                LOG.debug("State inserted into the database")
-
-                oauth_data_id = session.query(OAuthSession) \
-                    .filter(
-                        OAuthSession.state == new_state.state
-                        and
-                        OAuthSession.expires_at == new_state.expires_at
-                        and
-                        OAuthSession.code_verifier == new_state.code_verifier
-                        and
-                        OAuthSession.provider == new_state.provider
-                    ).first().id
-
-                LOG.debug("FETCHED STATE ID")
-                LOG.debug("State %s inserted successfully", state)
-                LOG.debug("verifier %s inserted sucessfully ", code_verifier)
-            return oauth_data_id
+
+                LOG.info("State inserted successfully.")
+
         except sqlite3.Error as e:
-            LOG.error(f"An error occurred: {e}")  # added here re1move
+            LOG.error(f"An error occurred: {e}")
             raise codechecker_api_shared.ttypes.RequestFailed(
                 codechecker_api_shared.ttypes.ErrorCode.AUTH_DENIED,
                 "OAuth data insertion failed. Please try again.")
@@ -242,17 +228,12 @@ def createLink(self, provider):
             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,
-                                             provider=provider)
-        if not oauth_data_id:
-            raise codechecker_api_shared.ttypes.RequestFailed(
-                codechecker_api_shared.ttypes.ErrorCode.AUTH_DENIED,
-                "OAuth data insertion failed.")
+        # inserting data in database
+        self.insertDataOauth(state=state,
+                             code_verifier=pkce_verifier,
+                             provider=provider)
 
-        LOG.info("Data inserted successfully with ID %s", oauth_data_id)
-        return url + "&oauth_data_id=" + str(oauth_data_id)
+        return url
 
     @timeit
     def performLogin(self, auth_method, auth_string):
@@ -286,8 +267,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]
-            oauth_data_id = parsed_query.get("oauth_data_id")[0]
-            # for code verifier from created link and original
             code_verifier_db = None
             state_db = None
             provider_db = None
@@ -299,7 +278,7 @@ def performLogin(self, auth_method, auth_string):
                                   OAuthSession.code_verifier,
                                   OAuthSession.provider,
                                   OAuthSession.expires_at) \
-                           .filter(OAuthSession.id == oauth_data_id) \
+                           .filter(OAuthSession.state == state) \
                            .first()
 
             if str(state_db) != state or str(provider_db) != provider \
@@ -344,7 +323,7 @@ def performLogin(self, auth_method, auth_string):
 
             # 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,

diff --git a/web/server/vue-cli/src/views/Login.vue b/web/server/vue-cli/src/views/Login.vue
@@ -219,9 +219,6 @@ export default {
         if (url) {
           this.success = false;
           this.error = false;
-          const params = new URLSearchParams(url);
-          localStorage.setItem("oauth_data_id",
-            params.get("oauth_data_id"));
 
           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
@@ -61,13 +61,11 @@ export default {
 
       if (params.code != null && params.state != null) {
 
-        const oauth_data_id = localStorage.getItem("oauth_data_id");
-        const fullUrl = `${url}&oauth_data_id=${oauth_data_id}`;
         this.$store
           .dispatch(LOGIN, {
             type: "oauth",
             provider: provider,
-            url: fullUrl
+            url: url
           })
           .then(() => {
             this.success = true;

diff --git a/web/tests/functional/authentication/oauth_server.py b/web/tests/functional/authentication/oauth_server.py
@@ -87,12 +87,10 @@ def login_tester(self):
                 if query_result:
                     state = params['state']
                     code = query_result['code']
-                    oauth_data_id = params['oauth_data_id']
                     code_challenge = params['code_challenge']
                     ccm = params['code_challenge_method']
                     return self.show_json({"code": code,
                                            "state": state,
-                                           "oauth_data_id": oauth_data_id,
                                            "code_challenge": code_challenge,
                                            "code_challenge_method": ccm})
 

diff --git a/web/tests/functional/authentication/test_authentication.py b/web/tests/functional/authentication/test_authentication.py
@@ -198,12 +198,11 @@ def try_login(self, provider, username, password):
             raise RequestFailed(data['error'])
 
         link = link.split('?')[0]
-        code, state, oauth_data_id, code_challenge, code_challenge_method = \
-            data['code'], data['state'], data['oauth_data_id'], \
+        code, state, code_challenge, code_challenge_method = \
+            data['code'], data['state'], \
             data['code_challenge'], \
             data['code_challenge_method']
         auth_string = f"{link}?code={code}&state={state}" \
-            f"&oauth_data_id={oauth_data_id}" \
             f"&code_challenge_method={code_challenge_method}" \
             f"&code_challenge={code_challenge}"