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

Protect data in the UserViewSet better #195

Open
2 tasks
sergei-maertens opened this issue Feb 10, 2021 · 3 comments
Open
2 tasks

Protect data in the UserViewSet better #195

sergei-maertens opened this issue Feb 10, 2021 · 3 comments
Labels
enhancement New feature or request

Comments

@sergei-maertens
Copy link
Contributor

I was just looking at the UserViewSet in the accounts because of a remark from Kelvin, and I think we should pro-actively do something to prevent data-exfiltration. Because of the SSO, as soon as you can log in to the ZAC, you can hit that endpoint and essentially get the list of all Utrecht staff accounts that have ever logged in.

If someone does that, I think it should:

  • be logged
  • possibly explicitly assign a user permission accounts:can-view-users

The endpoint exists for auto-complete purposes & for BPTL reading out some data from the ZAC for the process model. If we log it, we can audit it. If we assign user permissions, we prevent abuse - this would mean that only people that actively have to use the ZAC can hit that endpoint, rather than every single person with a Utrecht account

Acceptance criteria

  • GET requests to list/retrieve are logged (django-timeline-logger? we want persistent logging, not logs that dissapear because of the infrastructure configuration)
  • Add explicit permission and connect it to the permissions/authorizations machinery
@sergei-maertens sergei-maertens added the enhancement New feature or request label Feb 10, 2021
@CMasselink
Copy link

we prevent abuse - this would mean that only people that actively have to use the ZAC can hit that endpoint, rather than every single person with a Utrecht account

Shouldn't this be the basic principle for all access related questions? Users should only be able to use the application functions they need to do their job, nothing more (a bit depending on information classification).
If I don't have permissions to use the application, I shouldn't be able to use functions / endpoints of that application, regardless if SSO is implemented.

Is this a consequence of not implementing specific claims/scopes in SSO for this application?

@sergei-maertens
Copy link
Contributor Author

sergei-maertens commented Feb 11, 2021

Shouldn't this be the basic principle for all access related questions? Users should only be able to use the application functions they need to do their job, nothing more (a bit depending on information classification).

yes, we have those locked down now with data-specific permissions: https://github.com/GemeenteUtrecht/zaakafhandelcomponent/blob/main/backend/src/zac/core/permissions.py

For the case of the users endpoint, the API endpoint is not connected with a particular "application function". The application function relevant here is "autocomplete search of colleagues on name or username". The frontend implements this by calling a generic endpoint. When the endpoint is called, it does not know if this is in a auto-complete context, or if someone is just exfiltrating all the data. Either way, a permission needs to be defined to not just give access to the endpoint to any logged in user.

Is this a consequence of not implementing specific claims/scopes in SSO for this application?

Indirectly, yes. You could limit this that users need a particular ADFS group before they're allowed to log in, and then a LoginRequired permission is sufficient. However, the fact that users can log in to the application now means that we need more fine-grained permissions on what is otherwise sufficiently protected by requiring login.

For reasons of not-leaking-data, the current ADFS connection does not include the groups claim to the application, so there's no way for us now to make any assertions on user permissions/rights.

@CMasselink
Copy link

CMasselink commented Feb 11, 2021

I assume the auto-compleet function is used for things like assigning cases to a certain employee? I think auto-complete should only return names of employees who still have an active account and appropriate permissions in the application.

I think it is best to discuss with the DISO and security team if it is considered a problem if a user can view active users of an application by using a search endpoint. All employees have acccess to the Outlook Address book and can view all employees of the gemeente Utrecht.....

Update: and as discussed in Slack: I think it is best to implement ADFS in a way we can limit access to the application so LoginRequiered can be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants