openid / OpenID4VCI

60 stars 16 forks source link

Credential Issuer using multiple authorization servers #72

Closed Sakurann closed 8 months ago

Sakurann commented 10 months ago

In our implementation, Credential Issuer (RS) will use different Authorization Servers depending on the grant type: authorization code flow will use existing AS in production and there will be a new AS spun out for pre-auth code flow.

We did our best towards using the same AS but in the end it was impossible because we cannot make changes to the AS in production so that token endpoint accepts pre-auth code and using a new AS as an intermediary AS to the existing production-AS for authz code flow broke SSO and all other features (production AS is needed for authz code to authenticate existing customers). There was also a suggestion to use different credential issuer (RS) per AS, but most credentials can be issued using both of the grant types, so that would mean the wallet would have to determine which issuer to use depending on the grant type which adds a lot of complexity (i.e. making credential_issuer property in the credential offer an array so that the wallet can choose a right RS/issuer based on the flow?). We explored a lot of options and settled on using the same RS with two ASs.

This means that Credential Issuer will need to communicate in its metadata endpoints of multiple Authorization Servers, while currently issuer metadata only allows to communicate one. Here, we explored the following three options:

  1. defining authorization_code_authorization_server and pre-authorization_code_authorization_server parameters in the credential issuer metadata, the values of which are respective endpoints. (the parameter names already sound horrible, I know)
  2. defining authorization_servers parameter as an array of strings that can include multiple entries, the wallet will need to determine which AS to use looking into the metadata of each entry.
  3. defining authorization_servers parameter as an array of objects where each object entry clarifies which grant type which AS supports.

To the options 3, @selfissued rightly said this "gives the RS the job of supplying some AS metadata. That could easily become stale or out of sync with what the AS actually does. It's probably cleaner to let the AS speak for itself." but I think this comment also applies to option 1.

@jogu has pointed out that "others are likely to have use cases for multiple authorization servers other than an AS per grant type too.", while option 1 and 3 are pretty specific to AS per grant type.

option 2 is also an approach adopted in resource server metadata draft, which has recently been adopted by IETF OAuth WG. So I think option 2 is the best option. I did a draft PR #78 to see how it would look and I don't think it's too terrible. In most cases, I would expect authorization_servers array would only have one entry, but having this parameter as an array would be very helpful.

Feedback would be appreciated.

tlodderstedt commented 9 months ago

Let's assume we go with option 2. How is the wallet supposed to know it needs to pick an authorization by grant type? Is the assumption that the wallet inspects the AS metadata and picks one that has the technical capabilities it requires? So one AS would support pre-authz and the other one would support authz code, only? Is the assumption any of the ASs is able to handle the process for any user in the domain of the issuer? I'm asking since I have built an ecosystem where every AS managed a distinct set of users. In such a context, the wallet would need to go through a selection process, which needs to be informed by the user.

jogu commented 9 months ago

@tlodderstedt All good questions. https://www.ietf.org/archive/id/draft-ietf-oauth-resource-metadata-00.html#section-2 similarly doesn't say anything about how the authorization server to use is selected if there's multiple ones.

My assumption would be that the wallet goes through the list of ASes and figures out which ones are suitable for the wallet to use and meet any criteria for the particular request the wallet is going to make. If it still has more than one candidate it would probably need to ask the user which one to use.

However, having thought about this more, I think there's a fourth option:

  1. The issuer value for the authorization server to be used is included in the credential offer alongside the pre-authorized_code

This makes a lot of sense to me, as the issued authorization code will presumably only work with one authorization server, so explicitly telling the wallet to use that server seems much simpler.

tlodderstedt commented 9 months ago

all good as long as the assumption is any AS is able to handle all users relevant to the Issuer. Then selection can be based on the AS metadata.

Sakurann commented 9 months ago

I like the idea of combining @jogu 's suggestion with option 2. There are cases when the issuance flow starts without credential offer, so for those cases, option 4 alone would not be not sufficient. At the same time, to Torsten's point, the wallet would need to know which AS metadata parameters to use to pick an AS, so when different AS is to be used per grant type, option 4 would greatly simplify the work of the wallet. and if the wallet needs to pick AS using other AS metadata other than grant type, I assume that would needs to be defined by the wallet-AS ecosystem/trust framework..

Credential Offer could look like

{
   "credential_issuer": "https://credential-issuer.example.com",
   "credentials": [
      "UniversityDegree_JWT",
      {
         "format": "mso_mdoc",
         "doctype": "org.iso.18013.5.1.mDL"
      }
   ],
   "grants": {
      "authorization_code": {
         "issuer_state": "eyJhbGciOiJSU0Et...FYUaBy",
         "authorization_server": "https://authorization-server-1.com"
      },
      "urn:ietf:params:oauth:grant-type:pre-authorized_code": {
         "pre-authorized_code": "adhjhdjajkdkhjhdj",
         "user_pin_required": true,
         "authorization_server": "https://authorization-server-2.com"
      }
   }
}

with an metadata entry of

{
...
   "authorization_servers": [ "https://authorization-server-1.com", "https://authorization-server-2.com" ],
...
}
Sakurann commented 9 months ago

If AS is managing the distinct set of users, Joseph's suggestion could also help, if the Issuer can include the URL of a correct AS in the offer based on the user's actions that led to issuer_state/pre-auth code. if that is not possible, or there is no offer, I guess the wallet would need to know that for IssuerA, it needs to display to the user all of the entries in the authorization_servers array and use the one of the user choice..?

Sakurann commented 8 months ago

reporting back from discussions at IIW: there does not seem to be a strong opposition to adopting "authorization_servers": [] array structure.

There has been some concerns that majority of the implementations will have only one entry. As an alternative, it was suggested that implementers like Microsoft who have this kind of architecture with multiple ASs for one RS, can have a different path where issuer metadata is hosted per AS (e.g. issuer.com/pre-auth/.well-known/openid-credential-issuer and issuer.com/auth-code/.well-known/openid-credential-issuer) where `authorization_servers is a string. At least in Microsoft, we seriously considered this option, but came to a conclusion that it is not possible because it will not work for issuance during presentation, where wallet should be able to use one issuer identifier to start the issuance flow (in addition to the fact that duplicating every single metadata file is a lot of added complexity..)

so I think PR#78 that I did on this is ready for the WG to start reviewing.

selfissued commented 8 months ago

I support the ability to specify multiple authorization servers. Note that https://www.ietf.org/archive/id/draft-ietf-oauth-resource-metadata-01.html#name-protected-resource-metadata specifies authorization_servers.

Sakurann commented 8 months ago

yes, this new structure is largely inspired by that IETF draft ;)

Sakurann commented 8 months ago

PR merged