Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[AccessToken] A component for fetching and caching remote services access tokens#54013
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:7.4
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
First time I try to add a new component, may someone help me with this test failing ?
It seems that the added component tries to put an empty array in default framework configuration when unconfigured, whereas it should not, did I miss some function call in the configuration builder ? |
pounard commentedFeb 20, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
EDIT: And I was wrongly reading it, some are unrelated, some are my fault. Will fix it ASAP. |
If I understand properly, if this component is approved, everybody will be able to contribute to add bridge to any OAuth service in a similar way we can contribute to the Notifier/Mailer/Messenger component to add a bridge to a service? A common usage for this component can also be for Google/Facebook... sign implementation in I guess? I like it. |
I have a question about strict types, fabbot.iohttps://fabbot.io/report/symfony/symfony/54013/13a53c83f8401eac907e7fd712dca8105a9480b8 seems to consider as an error only 11 of them. Is this an error I should ignore (I think that the fabbot.io report is mixing two rules, one that says that declare() must be first in file, and the other that says that the licence header must be first in file). Does Symfony convention states that strict_types should not be used at all ? |
pounard commentedFeb 21, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Yes it's designed to be easily pluggable, that's the whole goal of it. FYI I tested the current OAuth2 implementation with Microsoft Azure OAuth, and it works. In the past and present, I had to deal with Salesforce OAuth2 as well (a lot), with the tiny exception that Salesforce requires some additional parameters (which can be arbitrarily user given in the actual state of code) it should work as well. The URL to DSN parsing we implemented passes along all unhandled parameters to the final endpoint URL, which allows that. Most OAuth2 providers that implements the OAuth standard as documented should work with the current implementation (you don't even have to add a bridge).
I'm not sure to understand what your are talking about, it's important to keep in mind this is for authorization flows that don't require user interaction, dedicated to B2B/MTM. |
Oh yes, sorry I read but forgot that part. |
5a76d9f
tocacd021
CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/AccessToken/Bridge/OAuth/ClientCredentials.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/AccessToken/Bridge/OAuth/ClientCredentialsProvider.phpShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/AccessToken/Manager/LockAccessTokenManagerDecorator.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
pounard commentedFeb 21, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@OskarStark Thank you very much ! |
7fb26f3
to7ca4507
CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
wow, massive body of work! i like it. the basic organization seems good to me, i only commented some minor details.
would be awesome to get a review by the maintainers of HWIOAuthBundle for this. ideally, this component would be useful for such a bundle and allow them to simplify what they provide. have you looked at HWIOAuthBundle to see if part of their OAuth implementation could be used here as well? (the license is MIT for both the bundle and symfony, so legally that should be no issue)
paging@stloyd :-)
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/AccessToken/Bridge/OAuth/ClientCredentialsProvider.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
@dbu thank you very much for your review, please ask any questions or don't hesitate to ask for more inline documentation when something doesn't seem clear enough to you ! |
8351a2a
tod981a14
Compare
I'll try rebasing this PR and submit it once again. If it doesn't get any traction I'll create a standalone bundle. But right now, I'm on hollydays so it'll wait septembre at least. |
Nice work.
The name of the component should reflect this. How does it fit with security’s |
Hi@pounard, That’s a very good idea and interesting work. I have been working with access tokens for a while now and I still think this is a missing piece in Symfony for securing MTM exchanges. I may have missed something about the concept of
In the example, you useclient ID/secret pair, but there are other credentials such asMutual TLS,JWT orSAML Assertions and certainly other custom/proprietary means. ❓ Question: is this component capable of generating adaquat requests for the given credentials |
If your input is an URL which has a recognized scheme which is registered through a credentials factory, then yes. The idea being in order to ease usage that the right provider can be guessed from the user-given credentials (which is arbitrarily designed to be represented as an URL with a scheme, which could be something as simple as
I'm not sure to understand your question here, but I'm guessing you are asking about once you fetched the access token, how to attach it to the remote service business request? If so, then it's actually up to you to decide how. The idea is to provide a way to fetch and cache token, not to use it. The Once again, this component doesn't give you any business code, simply a way to fetch and cache tokens. How they are been used is then your responsibility. Hence the paragraph about error management, each use case may rise all different sorts of exceptions, this API cannot predict which will be the kind of errors the API you call will raise.
Actually, no. The reason is explained in my previous answer. |
[AccessToken] fix licence yearfix
[AccessToken] apply cs fixes (again)[AccessToken] CS fixes[AccessToken] some pslam fixes (again)
…ng directly the AccessTokenManager service as a factory
f34e667
tod8b333e
Compare
Agree, but I'm not very good at naming things. Any suggestion is welcome! |
That is what Chat-GPT proposes:
|
Hum still not convinced with those names. |
I tried with Claude AI, and it ends up giving me |
is it about access or about authentication? i would not use the "Manager" suffix for a symfony component, the component is about something and has the models and some services to work with the models. |
What I can't understand is that the Security Component already includes the main concepts. Simply the naming is different, but the purpose remains the authentication of the user (user in the broad sense i.e. End User, server or any consumer of the application) using a token (provider agnostic).
Far be it from me to question all this work, because there are interesting concepts, but it seems a bit redundant with what is proposed by the Security Component |
pounard commentedSep 30, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Security component handles incoming user authentication in local app, whereas this proposed component handles authenticating to remote services, which is the other way around. I'm not sure that both share anything beside semantic actually. |
So it's basically a security component but for the client side? How about then:
|
Hello! I'm not forgetting this one, a lot of changes happened in my personal real life, so I had to slow down working for a few month. Aside the fact that a few people suggested that we should find a better name for the component and that it's still missing a few core contrib reviews and opinion, this PR is still fine as it is (considering that it should still pass tests). So, even if I have less time for this momentarily, it's still open to discussions, criticism and reviews, I'll welcome any comment and continue to read and respond! |
Uh oh!
There was an error while loading.Please reload this page.
Access Token component
Use case (TL;DR)
When you develop a Symfony project thatneeds to consume data from a remote service/API, inB2B (Business To Business) /MTM (Machine To Machine) mode,without any user interaction, in most cases you will need toauthenticate your HTTP requests for this remote service. It's rather common that this authentication is done by aBearer access token.
This component aims toprovide ways to configuring credentials then retrieve those tokens in a generic manner. It also aimsimprove performance improvement using caching for storing those access tokens for their lifetime, and toensure consistency by using locking around remote token fetch, in order to avoid parallel accesses to the remote endpoint when a token expires.
In this scenario, theSymfony application is a client, not a server, requesting tokens and consuming data, and not the other way around.
Rationale
This main purpose component is toprovide a common interface for fetching and refreshing remote access tokens, for webservices bearer tokens, SMTP XOAUTH, etc... as well as a some commonly used bridges.
The obvious use case is for managing OAuth2 token credentials, using the
client_credentials
andrefresh_token
grants, which are provided by a remote service and have short life time, in order to use them as aclient only. Theauthorization_code
flow is voluntarily excluded because it's meant to trigger a user interaction, which is out of scope right now (but may be later).It's important to keep this in mind, OAuth2 is a standard, and security wise very hazardous to implement by one self without using a production-ready, industry grade, well maintained specialized component.This component is not going to provide any crypto related or server side code: it's meant for B2B / MTM token exchange only. This component is not likely to introduce any security issue.
The main goal being to provide a token fetcher and manager generic API interface, which is not OAuth2 specific, thus may be used for various other kind of tokens. In real life, most token-based authentication seem to be inspired or variations of one of the OAuth2 grant flows.
Why would Symfony need this? For a few different reasons:
symfony/notifier
transports, especially theOrangeSmsTransport
that handles the token fetch by itself.symfony/mailer
component holds theXOAuth2Authenticator
whichonly accepts an hardcoded access token from configuration, without any refresh logic, which is carried by theDsn
instance upper in the stack. Tokens are almost always bound to a life time, sometimes a few minutes, sometimes a year, and those tokens cannot simply be set in configuration for this exact reason. Even if given via environment variables: changing it almost always in the current state requires a manual operation, or sometimes some services restart.Bearer
authentication or similar to authenticate into third party web servicesis a common task that all developer probably writes at least once per project.And why would it be better to have a component than doing it manually, like it is done in the
OrangeSmsTransport
? Well, for many reasons, one being that a few mistakes are common to see:OrangeSmsTransport
,it will fetch a token for each notification being sent, but the token it gets is valid for a certain amount of time:it's a waste of resources and a performance hog not to reuse it.symfony/mailer
Dsn
class use case, the configuration and the token will be cached in memory, which means that whenused through a service that lives during asymfony/messenger
bus process long-lived runtime, the token could get invalidated during runtime. Fixing it requires a restart. We can fix this by forcing the token to be systematically fetched for cache, validated, and refreshed programmatically, for each use.When you start to put all pieces altogether, it shows to be more complex than simply doing an HTTP request to fetch a token.In most real life scenarios we always want caching, validating and refreshing features to be implemented. This is even more true now that the
symfony/messenger
component is widely adopted by the community.My personal experience with this is that, in the company I work for, each project owns its own implementation of this lock, cache, validate and refresh algorithm. Sometimes more than once in a single project because we need to interconnect with various third party remote web services API, each having its own dialect for fetching tokens. That is the main reason why we want a generic, reusable and well-maintained component for this.We hope and do expect it will also be in the interest of many other people.
Proposed design
Main concepts
There the following concepts/interfaces which are really important here:
ProviderInterface
is what the bridge must implement: it has one method for fetching a new token, another one for refreshing an existing token. The abstract base implementation merges the two methods into a single fetch new token interface. This interface is hidden for the end user.CredentialsInterface
is a DTO that is typed and carries user credentials information, bridges must implement their credentials as well, this interface is exposed to the end user. Its only generic feature is to be able to produce a reproducible identifier for caching and locking.FactoryInterface
is what the bridge can implement in order to enable user-driven credentials configuration. From an URL schemes, it produces aCredentialsInterface
instance suitable for this component usage.AccessTokenManagerInterface
is the user facade, it provides the same two methods, but they are different in the sense that they only work with aCredentialsInterface
parameter.AccessTokenFetcher
is a service created from user YAML configuration, it is a replica ofAccessTokenManagerInterface
without the credentials parameter, since it already knows the user configuration.The access token manager works in this way:
Caching and locking
Caching and locking are done by decorating the
AccessTokenManagerInterface
.Caching should do:
CacheInterface
contract.Locking:
Note: for locking, there is probably ways to improve it, such as getting a read-only lock on first cache get, we are eager to hear your suggestions.
For this to work, cache and lock, we need for each credential a unique and reproducible identifier, that's why the only public visible method on
CredentialsInterface
isgetId(): string
. It's up to the implementation to compute a unique identifier.Note:
getId(): string
method could boil down to a__toString(): string
method which returns the URI (which is supposed to be unique). I have no strong opinion about this so all suggestions are welcome.In default configuration, lock decorates cache which decorates the default implementation in order to prevent concurrent cache accesses: a thread could fetch an invalid token from cache while another thread is fetching a new one, here this scenario is impossible. In case you prefer performance over consistency, it is possible, per configuration, to disable the lock completely.
Configuration and factories
In order to allow user configuration in YAML and environment, a piece of code is needed to glue user strings to
CredentialsInterface
instances, it's theFactoryInterface
:oauth
for all OAuth implementations, that requires agrant_type=
parameter in the URL to discriminate).During container compilation, each user given credentials in YAML configuration is parsed, in order to check for validality, then an associated service factory is registered that creates an
AccessTokenFetch
at runtime when requested.Usage and configuration
Overview
Component in its current state can be used with three different workflows:
AccessTokenManagerInterface
service: user handles its credentials configuration manually, creates aCredentialsInterface
implementation instance and passes it to the manager service for fetching the token. For example, sinceOrangeSmsTransport
already carries its credentials in the user configured DSN from the mailer component, it would be appropriate to keep it that way for backward compatibility and simplicity.access-token
component (which can be retrieved from environment variables like the database URL or SMTP DNS are) which is injected as aAccessTokenFetcher
instance into the container, one per configured credentials. This method would be preferable for users that need to extensively use this component for various services at once: it allows them to register all in configuration, pass credentials via environment variables, and keep their services implementations simple.access-token
component (which can be retrieved from environment variables like the database URL or SMTP DNS are) which is injected as aCredentialsInterface
instance into the container, one per configured credentials. The user can use theAccessTokenManagerInterface
by passing over the credentials that were injected.Using
AccessTokenManagerInterface
directlySimple API user code example using OAuth
client_credentials
grant flow:And that's pretty much it. Considering the user doesn't need to implement its own bridge and one of the core provided one fits the need.
Using YAML configuration
AccessTokenFetcher
Let's start by configuration:
The environment variable would look like:
Then, the same user code as upper would boil down to:
Implementing a bridge
Implementing a bridge consists in three different things:
CredentialsInterface
concrete implementations. This is mandatory.ProviderInterface
concrete implementations that deals with the associated credentials. This is mandatory.FactoryInterface
concrete implementations, that allows the credentials to be tied to a scheme and registered to allow the user to drive their configuration in YAML. This is optional, but if not implemented, cannot be YAML configured.About the current code state
It may be incomplete, nevertheless it's a fully functional, working, MVP. It holds the most important structural pieces and our opinion is a sufficiently good design to be used :
Per default, the choice has been made to prefer consistency over speed, in all cases, locking then cache access will be significantly faster than doing an HTTP request for fetching a token. In high throughput scenarios, this is probably a sensible default, for all others, it won't make a visible difference.
Naming can be wrong at some places, we are opened to any suggestion. Especially for shortening some interfaces names. Actual names are the most verbose and expressive names we could find, but they are great chances that you might have better ideas than us!
Known limitations
It's not possible, in the current state, to detect remote access token invalidity in a generic manner (e.g. you send the token to an SMTP server and it gets rejected) because the remote service implementation is opaque, errors can differ depending upon the remote service ("535 Authentication Unsucessful" for SMTP, "401 Authorization Required" for HTTP, etc... and which
\Exception
class ? It depends upon the user implementation or library in use). So, the final try/catch-error/refresh-token/retry algorithm must be implemented by the user, as shown in the previous example.DSN parsing is simplistic for now, and improvements will be done, in order, for example, to be able to combine HTTP Basic Auth with OAuth2 client credentials, this is quite common to stumble upon remote services requiring it.
What it is not
I'm repeating it, because it's important:
So now, what?
We are seeking for reviews, and will glady accept any opinion, any criticism. We do really hope this would get adopted, because this would improve:
Future plans
Note about dependency
In order to avoid creating hard dependencies to other components (notifier, mailer), there is two different ways I can see right now:
access-token
component, and inject it properly with some container compilation magic.