Implement OAuth2 authentication for API endpoints related to code components

balintbrews → credited penyaskito → .

My experience working with setting up oauth for Entity Share was not pleasant. I've written exhaustive docs for dealing with all the various error codes you can get. Despite setting it up multiple times for several sites in a distributed network of sites sharing content I still need to consult the docs and error codes every time. So I'd be pretty keen to see if we could implement another process here? If we have a cli here can't we prompt for a username and password, hit the user.login.http route and get a cookie then just use that to make further requests to the API? This would avoid us having to add another dependency.

I completely get the appeal of avoiding the complexity, but we will need to provide OAuth2 to follow industry standards, and to account for the use case when someone wants to manage components on multiple sites.

I still would like to add tests that make actual HTTP requests.

Besides that, we need to overcome some challenges with permissions. I naïvely thought when I created the issue that administer code components will cover everything we need. That's close, but not quite the full picture. Here is an overview from the perspective of API endpoints:

1. The biggest blocker is the _user_is_logged_in route requirement for the experience_builder.api.config.list route. That will break things in an OAuth-context. Maybe there is a way to remove it dynamically for those requests, but I wonder if we could handle that requirement differently.
2. Then there's the administer themes permission for the INDEX operation which not only is not the most intuitive choice, but it also returns all the data that can otherwise be accessed individually with a GET request, but for those the administer code components permission is needed.
3. The access administration pages permission also may not be intuitive either.

The key to potentially rethinking our permissions is that when using them in OAuth2 scopes, we need to have 1:1 relationships. A scope can't be associated with multiple permissions. We can have umbrella scopes holding other scopes, though.

I would love to simplify this and make it nicer for the scopes we provide in config. Any thoughts on how much we could easily update our permissions?

The key to potentially rethinking our permissions is that when using them in OAuth2 scopes, we need to have 1:1 relationships. A scope can't be associated with multiple permissions.

Is this a limitation of the dependencies we are using? I don't think it's a technical limitation by oauth2 itself.

#10: It is the limitation of the Simple OAuth module → , yes, but it doesn't change the fact that the permissions could use some love. We can always use umbrella scopes, so my ask is not necessarily to express everything with a single permission.

Let's focus on how we could:

1. improve the situation with administer themes and access administration pages — it doesn't feel right to map these to OAuth scopes in the context of those API operations;
2. remove the use of the _user_is_logged_in route requirement, and perhaps use a permission instead.

While the first one is more about semantics, the second one is potentially a hard blocker for this issue.

balintbrews → credited mglaman → .

Thanks to @mglaman, I now understand that I was having problems with the _user_is_logged_in access checker because of how my OAuth2 consumer was configured while testing. The client credentials grant type allows configuring a user to be used "as the author of all actions made". \Drupal\simple_oauth\Authentication\TokenAuthUser decorates the user account defined here, so when the _user_is_logged_in access checker calls isAuthenticated, it was checking that user. So this is expected, and won't be a blocker for this implementation, I just need to make sure (and document) that the anonymous user is not configured.

The administer themes permission will be handled in 🐛 Content authors cannot see components Active , and the access administration pages might also be going away based on some discussions.

So I'm good to proceed with the current permissions. I may rethink the scopes a bit.

This is a beautifully precise, well-tested MR! Kudos 🤩👏

I think @larowlan left a bunch of remarks worth addressing (especially the one regarding route definitions), and I think that this removing a single _permission: … from the "config delete" route (which technically should happen/have already happened in 📌 SdcController cleanup tasks Active ) would significantly simplify this MR — and would prevent this having to change again in the near future.

wim leers → credited effulgentsia → .

What a marvelous suggestion by @effulgentsia! 🤩

See #3529207-10: For selective reverting add DELETE auto-save endpoint → — we forgot to update CONTRIBUTING.md here 🙈 My bad! Will fix there.

Automatically closed - issue fixed for 2 weeks with no activity.