Skip to content

Commit

Permalink
Update OAuth strategy to be able to perform local testing (#962)
Browse files Browse the repository at this point in the history
  • Loading branch information
galvana authored Aug 1, 2022
1 parent 0952c6c commit bb063db
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/fidesops/api/v1/endpoints/oauth_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ def read_scopes() -> List[str]:
return SCOPE_REGISTRY


@router.post(OAUTH_CALLBACK, response_model=None)
@router.get(OAUTH_CALLBACK, response_model=None)
def oauth_callback(code: str, state: str, db: Session = Depends(get_db)) -> None:
"""
Uses the passed in code to generate the token access request
Expand Down
1 change: 1 addition & 0 deletions src/fidesops/core/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ class FidesopsConfig(FidesSettings):
is_test_mode: bool = os.getenv("TESTING", "").lower() == "true"
hot_reloading: bool = os.getenv("FIDESOPS__HOT_RELOAD", "").lower() == "true"
dev_mode: bool = os.getenv("FIDESOPS__DEV_MODE", "").lower() == "true"
oauth_instance: Optional[str] = os.getenv("FIDESOPS__OAUTH_INSTANCE")

class Config: # pylint: disable=C0115
case_sensitive = True
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from sqlalchemy.orm import Session

from fidesops.common_exceptions import FidesopsException, OAuth2TokenException
from fidesops.core.config import config
from fidesops.models.authentication_request import AuthenticationRequest
from fidesops.models.connectionconfig import ConnectionConfig
from fidesops.schemas.saas.saas_config import ClientConfig, SaaSRequest
Expand Down Expand Up @@ -76,6 +77,24 @@ def add_authentication(
request.headers["Authorization"] = "Bearer " + access_token
return request

@staticmethod
def _generate_state() -> str:
"""
Generates a string value to associate the authentication request
with an eventual OAuth2 callback response.
If an oauth_instance name is defined then the name is added as a
prefix to the generated state. The state prefix can be used by a
proxy server to route the callback response to a specific Fidesops
instance. This functionality is not part of the OAuth2 specification
but it can be used for local testing of OAuth2 workflows.
"""

state = str(uuid4())
if config.oauth_instance:
state = f"{config.oauth_instance}-{state}"
return state

@staticmethod
def _close_to_expiration(
expires_at: int, connection_config: ConnectionConfig
Expand Down Expand Up @@ -223,7 +242,7 @@ def get_authorization_url(
self._check_required_secrets(connection_config)

# generate the state that will be used to link this authorization request to this connector
state = str(uuid4())
state = self._generate_state()
AuthenticationRequest.create_or_update(
db, data={"connection_key": connection_config.key, "state": state}
)
Expand Down
6 changes: 3 additions & 3 deletions tests/ops/api/v1/endpoints/test_oauth_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ def callback_url(self) -> str:
return V1_URL_PREFIX + OAUTH_CALLBACK

def test_callback_for_missing_state(self, db, api_client: TestClient, callback_url):
response = api_client.post(
response = api_client.get(
callback_url, params={"code": "abc", "state": "not_found"}
)
assert response.status_code == 404
Expand All @@ -476,7 +476,7 @@ def test_callback_for_valid_state(
"state": "new_request",
},
)
response = api_client.post(
response = api_client.get(
callback_url, params={"code": "abc", "state": "new_request"}
)
assert response.ok
Expand Down Expand Up @@ -505,7 +505,7 @@ def test_callback_for_valid_state_with_token_error(
"state": "new_request",
},
)
response = api_client.post(
response = api_client.get(
callback_url, params={"code": "abc", "state": "new_request"}
)
assert response.status_code == 400
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,20 +166,22 @@ def test_oauth2_authentication_failed_refresh(


class TestAuthorizationUrl:
@mock.patch("fidesops.service.authentication.authentication_strategy_oauth2.uuid4")
@mock.patch(
"fidesops.service.authentication.authentication_strategy_oauth2.OAuth2AuthenticationStrategy._generate_state"
)
@mock.patch(
"fidesops.models.authentication_request.AuthenticationRequest.create_or_update"
)
def test_get_authorization_url(
self,
mock_create_or_update: Mock,
mock_uuid: Mock,
mock_state: Mock,
db: Session,
oauth2_connection_config,
oauth2_configuration,
):
state = "unique_value"
mock_uuid.return_value = state
mock_state.return_value = state
auth_strategy: OAuth2AuthenticationStrategy = get_strategy(
"oauth2", oauth2_configuration
)
Expand Down

0 comments on commit bb063db

Please sign in to comment.