Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PB-1109: Add PUT endpoint to update user #37

Merged
merged 10 commits into from
Nov 13, 2024

Conversation

asteiner-swisstopo
Copy link
Contributor

@asteiner-swisstopo asteiner-swisstopo commented Nov 5, 2024

Adds PUT endpoint users/{username} to the the REST API of service-control that allows you to update a user.

Summary of changes:

  1. Add new PUT endpoint users/{username} that returns HTTP status code

    • 200 (OK) if the user has been updated successfully
    • 404 (Not Found) if there is no user with the given username
    • 500 (Internal Server Error) if there is an inconsistency with Cognito or if there is no provider with the given ID
      ⚠️ This is is the only difference in behaviour to the DELETE in Add DELETE endpoint for users #34
    • 503 (Service Unavailable) if Cognito cannot be reached

    ℹ️ This uses PUT and not PATCH because we deem it good enough to start with and it's supposedly easier to implement. It's a bit annoying maybe that the user has to specify all fields, even those she/he does not want to change. But then again they can those values via the GET.

  2. Rename cognito.utils.user.update_user to update_cognito_user to put more clearly indicate that this is about Cognito and not about the User model in Django. See the discussion in Add DELETE endpoint for users #34.

Example payload to PUT users/{username}:

{
    "username": "dude",
    "first_name": "Jeff",
    "last_name": "Bridges",
    "email": "[email protected]",
    "provider_id": 123,
}

Example response if provider does not exist:

{
    "code": 500,
    "description": "Provider does not exist"
}

app/cognito/utils/user.py Show resolved Hide resolved
app/access/api.py Outdated Show resolved Hide resolved
Copy link
Contributor

@schtibe schtibe left a comment

Choose a reason for hiding this comment

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

Just small comments, looking good otherwise 👍

app/access/api.py Outdated Show resolved Hide resolved
app/access/api.py Outdated Show resolved Hide resolved
app/access/api.py Outdated Show resolved Hide resolved
@asteiner-swisstopo asteiner-swisstopo merged commit 3cfedb9 into develop Nov 13, 2024
3 checks passed
@asteiner-swisstopo asteiner-swisstopo deleted the feat-PB-1109-update-user-endpoint branch November 13, 2024 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants