You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "betodealmeida (via GitHub)" <gi...@apache.org> on 2024/04/03 15:40:58 UTC

[PR] chore(OAuth2): refactor for custom OAuth2 clients [superset]

betodealmeida opened a new pull request, #27880:
URL: https://github.com/apache/superset/pull/27880

   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   
   This PR is a small refactor of https://github.com/apache/superset/pull/27631, addressing a different workflow for OAuth2 suggested by @villebro, where any number of OAuth2 clients can be configured and assigned manually to databases.
   
   The main change is that the DB engine spec no longer inspects the application config to get the OAuth2 client details (ID, secret, etc.). Instead, the OAuth2 related methods in the DB engine spec now receive a configuration object:
   
   ```python
   class OAuth2ClientConfig(TypedDict):
       id: str
       secret: str
       scope: str
       redirect_uri: str
       authorization_request_uri: str
       token_request_uri: str
   ```
   
   The configuration is read from the **database model**:
   
   ```python
   oauth2_config = database.get_oauth2_config()
   ```
   
   The current implementation simply builds the config object from `superset_config.py`, but in the future the database could check first for a custom OAuth2 client associated with it and use that instead. Similarly, to check if OAuth2 is enabled in a given database we now do:
   
   ```python
   if database.is_oauth2_enabled():
      pass
   ```
   
   The current method also simply checks `superset_config.py` for now.
   
   Having a standard configuration being passed to the DB engine spec has helped simplify the OAuth2 related methods. I was able to move the methods from GSheets to the base DB engine spec by adding a few new attributes. All that is needed to support OAuth2 in Gsheets now is:
   
   ```python
   from shillelagh.adapters.api.gsheets.lib import SCOPES
   
   class GSheetsEngineSpec(ShillelaghEngineSpec):
   
       supports_oauth2 = True
       oauth2_scope = " ".join(SCOPES)
       oauth2_authorization_request_uri = "https://accounts.google.com/o/oauth2/v2/auth"
       oauth2_token_request_uri = "https://oauth2.googleapis.com/token"
       oauth2_exception = UnauthenticatedError
   
       @classmethod
       def get_url_for_impersonation(
           cls,
           url: URL,
           impersonate_user: bool,
           username: str | None,
           access_token: str | None,
       ) -> URL:
           if impersonate_user and access_token:
               url = url.update_query_dict({"access_token": access_token})
   
           return url
   ```
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org