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

Productize app tokens for service integration #10292

Closed
butonic opened this issue Oct 14, 2024 · 8 comments · Fixed by #10550
Closed

Productize app tokens for service integration #10292

butonic opened this issue Oct 14, 2024 · 8 comments · Fixed by #10550
Assignees

Comments

@butonic
Copy link
Member

butonic commented Oct 14, 2024

We have an initial implementation for app tokens, but it is disabled by default and marked as experimental.

App tokens are intended to be generated by end users so they can integrate legacy tools that do not support OIDC. However, the current implementation cannot scope the token which is one of the reasons why we marked this as experimental.

There is another use case that we can productize already. The admin can generate app tokens for system accounts used by external services that can then interact with the graph api, e.g. to manage space membership.

The latter does not need to expose the token generation endpoint, as only admins can generate tokens. A PR for the helm chart that adds this as a feature is in owncloud/ocis-charts#767

@butonic
Copy link
Member Author

butonic commented Oct 14, 2024

cc @tbsbdr @dragotin @micbar @wkloucek

The code to support this scope / use case is implemented and documented but needs a security review.

@tbsbdr
Copy link
Contributor

tbsbdr commented Oct 16, 2024

@micbar @dragotin I consider app tokens to be extremely important. I propose to scope app tokens for the next qa-sprint (from what we know as of today: do we still need development or solely security-qa?) do you agree to scope app tokens in the next sprint?

@micbar micbar moved this from Qualification to Prio 2 in Infinite Scale Team Board Oct 21, 2024
@micbar
Copy link
Contributor

micbar commented Oct 21, 2024

  • Needs a security review.

@micbar
Copy link
Contributor

micbar commented Nov 11, 2024

Security Code Review

Tasks

  • Do a code review on ocis and reva code checking for obviuos issues
  • Check that the config switch to "allow impersonation" is correctly implemented and cannot be abused
  • Check the the tokens are not usable to access data of other users

@kobergj @DeepDiver1975 Please add more if needed.

@2403905
Copy link
Contributor

2403905 commented Nov 11, 2024

The impersonation API uses the userId or userName to generate the token and doesn't validate if both represent the same user. The userId has priority over the userName. The scammers could provide Admin with their own name and the victim's userID to generate the token and get access to the victim's data.

The request below contains moss's ID and einstein user name.
https://localhost:9200/auth-app/tokens?userID=058bff95-6708-4fe5-91e4-9ea3d377588b&userName=einstein&expiry=10m

The API response doesn't contain any information that allows Admin to determine who the token bearer is.
{"token":"158YL34R7U6NMPn0","expiration_date":"2024-11-11T18:57:19.362512+01:00","created_date":"2024-11-11T18:55:19+01:00","label":"Generated via Impersonation API"}

@2403905 2403905 self-assigned this Nov 12, 2024
@github-project-automation github-project-automation bot moved this from In progress to Done in Infinite Scale Team Board Nov 13, 2024
@micbar micbar reopened this Nov 13, 2024
@github-project-automation github-project-automation bot moved this from Done to In progress in Infinite Scale Team Board Nov 13, 2024
@micbar
Copy link
Contributor

micbar commented Nov 13, 2024

Needs more info before closing.

@2403905
Copy link
Contributor

2403905 commented Nov 13, 2024

Checklist:

  • Checked the CLI
  • Checked if the impersonation API works only when the AUTH_APP_ENABLE_IMPERSONATION=true
  • Checked if only users with admin rights can issue a key via the impersonation API.
  • Checked if the token expiration works properly.
  • Checked if access was denied to another user's personal space and that wasn't shared with the key owner.

@micbar
Copy link
Contributor

micbar commented Nov 18, 2024

@mmattel Fine from ENG POV, could you check if the dev docs are sufficient?

@micbar micbar moved this from In progress to Done in Infinite Scale Team Board Nov 18, 2024
@micbar micbar closed this as completed Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants