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

Update OAuth scopes #143

Closed
andregasser opened this issue Feb 12, 2023 · 2 comments · Fixed by #382
Closed

Update OAuth scopes #143

andregasser opened this issue Feb 12, 2023 · 2 comments · Fixed by #382
Assignees
Milestone

Comments

@andregasser
Copy link
Owner

The Mastodon OAuth Scopes page specifies additional scopes for push and admin related scopes. We should at least provide these scopes as well:

  • push

The follow scope has been deprecated in Mastodon 3.5.0 and we should mark it eventually. Users should start to request read and/or write scope instead.

The admin scopes admin:read and admin:write can be left aside for the moment, as we do not implement any of the admin calls yet.

@bocops
Copy link
Collaborator

bocops commented Dec 6, 2023

I'd like to do this, including the addition of both admin scopes and all child scopes and also the deprecation of our definition of FOLLOW.

Regarding our definition of ALL(Scope(READ, WRITE, FOLLOW).toString()), this is currently a convoluted way of defining ALL.scopeName as read write follow. Instead of changing this definition to something like read write push, I think it would be sensible to just deprecate this one as well, while keeping its scopeName as currently defined. In the future, this should be removed with FOLLOW.

Reason: Redefining ALL might lead to hard-to-detect errors, for example if a library user has previously used that scope for app creation but not yet in a specific authorization request. According to Mastodon documentation about OAuth scopes:

The set of scopes saved during app creation must include all the scopes that you will request in the authorization request, otherwise authorization will fail.

In the future, anyone requesting full access should just do this by something like Scope(READ, WRITE, PUSH) directly.

Any objections?

@bocops bocops added this to the 2.0.0 milestone Dec 6, 2023
@bocops
Copy link
Collaborator

bocops commented Dec 6, 2023

Also marking this as necessary for 2.0.0. We do have some admin methods by now, but lack the possibility for users to declare the scopes necessary to access them.

@bocops bocops self-assigned this Dec 7, 2023
bocops added a commit to bocops/bigbone that referenced this issue Dec 8, 2023
- makes all scope parameters in OAuth and App methods optional. Endpoints not receiving a scope parameter will default to granting "read" permission.
- removes default parameter value from Scope constructor. Users will need to define a certain scope if they want to use it.
Closes andregasser#116.

- adds all scopes as defined by Mastodon, marks existing scopes FOLLOW and ALL as deprecated.
- removes all internal usage of the now deprecated ALL scope.
Closes andregasser#143.

- moves deduplication of scopes from a separate function into the one building the parameter string. This avoids unnecessarily throwing an IllegalArgumentException.
Closes andregasser#381.
@bocops bocops mentioned this issue Dec 8, 2023
5 tasks
PattaFeuFeu added a commit that referenced this issue Dec 15, 2023
* Update scopes used in OAuth

- makes all scope parameters in OAuth and App methods optional. Endpoints not receiving a scope parameter will default to granting "read" permission.
- removes default parameter value from Scope constructor. Users will need to define a certain scope if they want to use it.
Closes #116.

- adds all scopes as defined by Mastodon, marks existing scopes FOLLOW and ALL as deprecated.
- removes all internal usage of the now deprecated ALL scope.
Closes #143.

- moves deduplication of scopes from a separate function into the one building the parameter string. This avoids unnecessarily throwing an IllegalArgumentException.
Closes #381.

* Enhance Scope usage in tests

- in integration tests, declare "full scope" once
- in unit tests, do not specify any Scope unless directly tested
- remove deprecated "follow" scope from the access token asset

* In samples to get access token, ensure that scopes requested for the token match those in OAuth URL.

* Enable hierarchical access to defined scopes

- replace Name enum with hierarchical structure for a more intuitive access to individual scopes
- changing samples and USAGE.md to match

Examples:
- Scope(Scope.Name.READ) becomes Scope(Scope.READ.ALL)
- Scope(Scope.Name.ADMIN_READ_ACCOUNTS) becomes Scope(Scope.ADMIN.READ.ACCOUNTS)

---------

Co-authored-by: Patrick Geselbracht <code@patrick-geselbracht.eu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants