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.

Sign up for GitHub

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

Jump to bottom

Change read:me scope to profile scope #30357

Merged
merged 8 commits into from
Jun 6, 2024
Merged

Change read:me scope to profile scope #30357

merged 8 commits into from
Jun 6, 2024

Conversation

ThisIsMissEm
Copy link
Contributor

This would resolve #30355, though introduced a quirk with the scope parser logic, which I'm not sure I've solved in an acceptable manner?

Basically the profile scope would otherwise default to all which is read and write, but for profile it will only ever be read.

Screenshot 2024-05-18 at 01 31 03

@renchap renchap added this to the 4.3.0 milestone May 20, 2024
ClearlyClaire
ClearlyClaire reviewed May 20, 2024
View reviewed changes
Copy link
Contributor

@ClearlyClaire ClearlyClaire left a comment

Choose a reason for hiding this comment

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

How does it affect existing registrations with read:me?

config/locales/doorkeeper.en.yml Outdated Show resolved Hide resolved
@ThisIsMissEm
Copy link
Contributor Author

How does it affect existing registrations with read:me?

There shouldn't be any yet, given it's only in bleeding-edge main, and isn't documented beyond me saying it's "coming in 4.3"

We could always do a migration on oauth_applications and oauth_tokens to rewrite read:me in scopes to profile but I'm hoping that's not necessary.

@ClearlyClaire
Copy link
Contributor

There shouldn't be any yet, given it's only in bleeding-edge main, and isn't documented beyond me saying it's "coming in 4.3"

It's been merged roughly a month ago, and two weeks ago you made it the default for new apps created through the settings interface.

Looking into mastodon.social's database, there are 4 applications with read:me as a scope.

@ClearlyClaire ClearlyClaire changed the title refactor: change read:me scope to profile scope Change read:me scope to profile scope May 21, 2024
@ClearlyClaire ClearlyClaire changed the title Change read:me scope to profile scope Change read:me scope to profile scope May 21, 2024
@ThisIsMissEm
Copy link
Contributor Author

There shouldn't be any yet, given it's only in bleeding-edge main, and isn't documented beyond me saying it's "coming in 4.3"

It's been merged roughly a month ago, and two weeks ago you made it the default for new apps created through the settings interface.

Looking into mastodon.social's database, there are 4 applications with read:me as a scope.

Oh! Good point! I'll add database migrations to this PR then; basically a find scopes LIKE %read:me% and then each in batches to rewrite?

@ThisIsMissEm
Copy link
Contributor Author

Have updated with the migration (I made it reversible just in case)

@ClearlyClaire ClearlyClaire force-pushed the fix/change-read-me-to-profile-scope branch from 5032d8b to 47dbe08 Compare June 5, 2024 12:59
@ThisIsMissEm
Copy link
Contributor Author

@ClearlyClaire your changes make sense to me!

@ClearlyClaire ClearlyClaire requested a review from a team June 5, 2024 13:15
renchap
renchap approved these changes Jun 5, 2024
View reviewed changes
Copy link
Sponsor Member

@renchap renchap left a comment

Choose a reason for hiding this comment

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

👍

This will need a documentation PR.

@ThisIsMissEm
Copy link
Contributor Author

👍

This will need a documentation PR.

@renchap already started it: mastodon/documentation#1445

@ClearlyClaire ClearlyClaire added this pull request to the merge queue Jun 6, 2024
Merged via the queue into mastodon:main with commit e02d23b Jun 6, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@renchap renchap renchap approved these changes

@ClearlyClaire ClearlyClaire ClearlyClaire approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.3.0
Development

Successfully merging this pull request may close these issues.

Rename read:me scope to "profile"
3 participants
@ThisIsMissEm @ClearlyClaire @renchap