Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Added Auth0 Connector #991

Merged
merged 11 commits into from
Aug 3, 2022
Merged

Added Auth0 Connector #991

merged 11 commits into from
Aug 3, 2022

Conversation

ghost
Copy link

@ghost ghost commented Jul 29, 2022

Purpose

  • Purpose of this PR is to add Auth0 connector User access and erasure endpoints

Changes

  • Added config for Auth0 User Access and Erasure Endpoints
  • Added related dataset and configuration
  • Added associated tests and fixtures to validate above functionality

Note

Checklist

  • Update CHANGELOG.md file
    • Merge in main so the most recent CHANGELOG.md file is being appended to
    • Add description within the Unreleased section in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.
    • Add a link to this PR at the end of the description with the PR number as the text. example: #1
  • Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram.
  • If docs updated (select one):
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Good unit test/integration test coverage
  • This PR contains a DB migration. If checked, the reviewer should confirm with the author that the down_revision correctly references the previous migration before merging
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

Ticket

Fixes #796

@ghost ghost requested a review from galvana July 29, 2022 18:57
@galvana galvana added the run unsafe ci checks Triggers running of unsafe CI checks label Aug 1, 2022

test_request:
method: GET
path: /api/v2/[email protected]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this would work, we should define the query parameters using the query_params property.

path: /api/v2/users-by-email
query_params:
  - name: email
    value: [email protected]

yield user

# Deleting user after verifying update request
headers = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to redefine the headers since we've already defined them on line 125.

}
headers = {"Authorization": f"Bearer {auth0_secrets['access_token']}"}
users_response = requests.post(
url=f"{base_url}/dbconnections/signup", json=body, headers=headers
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we using this /dbconnections endpoint instead of something like /api/v2/users?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both endpoints can be used to create user but I have updated code to use /api/v2/users

}

user_delete_response = requests.delete(
url=f"{base_url}/api/v2/users/auth0|{auth0_erasure_identity_email}",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this delete URL correct? It looks like there might be a typo because of the | character.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This worked fine as we needed to add /api/v2/users/<auth|id> but I have updated code not to contain '|' character in url.

url=f"{base_url}/dbconnections/signup", json=body, headers=headers
)
user = users_response.json()
assert 200 == users_response.status_code
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use assert users_response.ok or the HTTP_200_OK constant from starlette.status.

error_message = (
f"User with email {auth0_erasure_identity_email} could not be added to auth0"
)
poll_for_existence(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the user actually take some time to show up on the Auth0?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It usually doesn't take more than 1 seconds, I still added poll_for_existence just to make sure user is created and available.

"""Full erasure request based on the auth0 SaaS config"""

privacy_request = PrivacyRequest(
id=f"test_saas_erasure_request_task_{random.randint(0, 1000)}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change the id to test_auth0_erasure_request_task so we can uniquely identify it.

) -> None:
"""Full access request based on the Auth0 SaaS config"""
privacy_request = PrivacyRequest(
id=f"test_saas_access_request_task_{random.randint(0, 1000)}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change the id to test_auth0_access_request_task so we can uniquely identify it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good first round, I just have some questions around a few things before I can approve. It also looks like the user_logs endpoint isn't returning any data at the moment.
user_logs are deleted after some days which is why it returns empty. We can verify it by changing password of identity user, then we will get user logs.

Copy link
Collaborator

@galvana galvana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good first round, I just have some questions around a few things before I can approve. It also looks like the user_logs endpoint isn't returning any data at the moment.

@galvana galvana added run unsafe ci checks Triggers running of unsafe CI checks and removed run unsafe ci checks Triggers running of unsafe CI checks labels Aug 2, 2022
Copy link
Collaborator

@galvana galvana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the requested changes, this looks good to go!

@galvana galvana merged commit f98b302 into main Aug 3, 2022
@galvana galvana deleted the 796-saas-connector-auth0 branch August 3, 2022 00:38
sanders41 pushed a commit that referenced this pull request Sep 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
run unsafe ci checks Triggers running of unsafe CI checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auth0 - connector framework
1 participant