diff --git a/cura/OAuth2/AuthorizationRequestHandler.py b/cura/OAuth2/AuthorizationRequestHandler.py index 83b94ed586..b002039491 100644 --- a/cura/OAuth2/AuthorizationRequestHandler.py +++ b/cura/OAuth2/AuthorizationRequestHandler.py @@ -25,6 +25,8 @@ class AuthorizationRequestHandler(BaseHTTPRequestHandler): self.authorization_callback = None # type: Optional[Callable[[AuthenticationResponse], None]] self.verification_code = None # type: Optional[str] + self.state = None # type: Optional[str] + # CURA-6609: Some browser seems to issue a HEAD instead of GET request as the callback. def do_HEAD(self) -> None: self.do_GET() @@ -58,7 +60,14 @@ class AuthorizationRequestHandler(BaseHTTPRequestHandler): # \return HTTP ResponseData containing a success page to show to the user. def _handleCallback(self, query: Dict[Any, List]) -> Tuple[ResponseData, Optional[AuthenticationResponse]]: code = self._queryGet(query, "code") - if code and self.authorization_helpers is not None and self.verification_code is not None: + state = self._queryGet(query, "state") + if state != self.state: + token_response = AuthenticationResponse( + success = False, + err_message=catalog.i18nc("@message", + "The provided state is not correct.") + ) + elif code and self.authorization_helpers is not None and self.verification_code is not None: # If the code was returned we get the access token. token_response = self.authorization_helpers.getAccessTokenUsingAuthorizationCode( code, self.verification_code) diff --git a/cura/OAuth2/AuthorizationRequestServer.py b/cura/OAuth2/AuthorizationRequestServer.py index 51a8ceba77..d1cf38fa62 100644 --- a/cura/OAuth2/AuthorizationRequestServer.py +++ b/cura/OAuth2/AuthorizationRequestServer.py @@ -25,3 +25,6 @@ class AuthorizationRequestServer(HTTPServer): ## Set the verification code on the request handler. def setVerificationCode(self, verification_code: str) -> None: self.RequestHandlerClass.verification_code = verification_code # type: ignore + + def setState(self, state: str) -> None: + self.RequestHandlerClass.state = state diff --git a/cura/OAuth2/AuthorizationService.py b/cura/OAuth2/AuthorizationService.py index 0848623410..13e0e50373 100644 --- a/cura/OAuth2/AuthorizationService.py +++ b/cura/OAuth2/AuthorizationService.py @@ -153,13 +153,15 @@ class AuthorizationService: verification_code = self._auth_helpers.generateVerificationCode() challenge_code = self._auth_helpers.generateVerificationCodeChallenge(verification_code) + state = AuthorizationHelpers.generateVerificationCode() + # Create the query string needed for the OAuth2 flow. query_string = urlencode({ "client_id": self._settings.CLIENT_ID, "redirect_uri": self._settings.CALLBACK_URL, "scope": self._settings.CLIENT_SCOPES, "response_type": "code", - "state": "(.Y.)", + "state": state, # Forever in our Hearts, RIP "(.Y.)" (2018-2020) "code_challenge": challenge_code, "code_challenge_method": "S512" }) @@ -168,7 +170,7 @@ class AuthorizationService: QDesktopServices.openUrl(QUrl("{}?{}".format(self._auth_url, query_string))) # Start a local web server to receive the callback URL on. - self._server.start(verification_code) + self._server.start(verification_code, state) ## Callback method for the authentication flow. def _onAuthStateChanged(self, auth_response: AuthenticationResponse) -> None: diff --git a/cura/OAuth2/LocalAuthorizationServer.py b/cura/OAuth2/LocalAuthorizationServer.py index a80b0deb28..780adf54aa 100644 --- a/cura/OAuth2/LocalAuthorizationServer.py +++ b/cura/OAuth2/LocalAuthorizationServer.py @@ -36,7 +36,8 @@ class LocalAuthorizationServer: ## Starts the local web server to handle the authorization callback. # \param verification_code The verification code part of the OAuth2 client identification. - def start(self, verification_code: str) -> None: + # \param state The unique state code (to ensure that the request we get back is really from the server. + def start(self, verification_code: str, state: str) -> None: if self._web_server: # If the server is already running (because of a previously aborted auth flow), we don't have to start it. # We still inject the new verification code though. @@ -53,6 +54,7 @@ class LocalAuthorizationServer: self._web_server.setAuthorizationHelpers(self._auth_helpers) self._web_server.setAuthorizationCallback(self._auth_state_changed_callback) self._web_server.setVerificationCode(verification_code) + self._web_server.setState(state) # Start the server on a new thread. self._web_server_thread = threading.Thread(None, self._web_server.serve_forever, daemon = self._daemon)