OAI / OpenAPI-Specification

The OpenAPI Specification Repository
https://openapis.org
Apache License 2.0
29.11k stars 9.08k forks source link

Representation of Client-Initiated Backchannel Authentication (CIBA) in 3.2.0 #4106

Open SensibleWood opened 2 months ago

SensibleWood commented 2 months ago

I'd like to raise a concern with including CIBA in the OAuth Flow Object in 3.2.0.

I understand that CIBA can, feasibly, be implemented using bare OAuth 2.0 but realistically it's an OpenID Connect profile. A user is correlated by the Authorization Server using login_hint, id_token_hint, etc - these are OpenID Connect features.

I think there needs to be a separation-of-concerns between OAuth 2.0 and OpenID Connect in OpenAPI. The semantics of OpenID Connect provides proofs-of-authentication, with support for request objects, ID tokens, et al. OpenID Connect obviously builds on top of OAuth, but if we go down the road of starting to bundle OpenID Connect profiles into the OAuth Flow it has the potential to get pretty messy .

I propose an alternative approach:

I think that if we did it this way we can avoid duplicating OpenID Connect Discovery metadata in OpenAPI, and instead allow that to live where it needs to i.e. in the discovery endpoint. Implementing OpenID Discovery is a common practice, so I think we are in a good place to leverage an existing, external metadata source.

The descriptions - and maybe a nice JSON Schema as an addendum that we provide to be helpful - will then help tooling makers make sense of the discovery metadata from the lens of OpenAPI.

Happy to get involved if this approach is perceived as a good idea. I know OpenID in enough to detail to be vaguely useful.

handrews commented 2 months ago

Paging @shilpa-padgaonkar

SensibleWood commented 2 months ago

Just thinking about this a bit more. We could have an object as follows:

HsbcCibaProfile:
  type: openIdConnect
  openIdConnectUrl: https://sandbox.ob.hsbcnet.com/.well-known/openid-configuration
  profile: CIBA

Where CIBA is an instruction set that defines what CIBA "means" for a user and how to process the information in the well-known endpoint.

Similarly:

HsbcFapi2Profile:
  type: openIdConnect
  openIdConnectUrl: https://sandbox.ob.hsbcnet.com/.well-known/openid-configuration
  profile: "FAPI2.0"

If I was going to get really creative I'd probably have a bunch of JSON Paths available that could be used to select from the discovery document, and where identified by the profile value:

{
  "FAPI2.0": {
    "grantTypes": "$.response_types_supported[?(@ == 'code')]"
  }
}

And only if all of those were not null could the profile be resolved. Probably stepping into implementation territory here, but the "processing rules" approach is way we could bridge the gap between what OpenAPI does and what OpenID Connect does and how we can source information from the correct place.

LasneF commented 2 months ago

@SensibleWood there is this https://github.com/OAI/OpenAPI-Specification/pull/3615 that is cooking

it is merged but branch 3.2 is not public yet

What would be your feedback about it ?

SensibleWood commented 2 months ago

@SensibleWood there is this #3615 that is cooking

it is merged but branch 3.2 is not public yet

What would be your feedback about it ?

@LasneF well basically everything I said the 1st comment - in summary, CIBA is an OpenID Connect profile and mixing this into OAuth 2.0 provides no separation-of-concerns between the two. The words there reflects my understanding of the changes you've highlighted in that #3615. All subsequent comments reflect my concerns

handrews commented 2 months ago

@SensibleWood as I've worked on diagraming the OAS (including everything that's already included in or plausibly proposed for 3.2), I've noticed that the new CIBA fields add a lot to the OpenAPI Flow Object.

This is the part of the OAS (and APIs) that I understand the least, but from what I do understand, I like the direction.

SensibleWood commented 2 months ago

I've noticed that the new CIBA fields add a lot to the OpenAPI Flow Object.

@handrews and therein lies the issue. They do add more information, but it is not necessarily the right way to convey the information and is only a subset of the possible information on offer.

@handrews @LasneF @shilpa-padgaonkar I just want to make my thoughts on the approach to OpenID Connect profiles absolutely clear. I feel like a worked example is in order to make my point.

The sample OpenID Connect Discovery URL above points to this data:

{
  "version": 1.0,
  "issuer": "https://secure.sandbox.ob.hsbcnet.com",
  "authorization_endpoint": "https://sandbox.ob.hsbcnet.com/obie/open-banking/v1.1/oauth2/authorize",
  "registration_endpoint": "https://secure.sandbox.ob.hsbcnet.com/obie/open-banking/v3.2/oauth2/register",
  "token_endpoint": "https://secure.sandbox.ob.hsbcnet.com/obie/open-banking/v1.1/oauth2/token",
  "jwks_uri": "https://sandbox.ob.hsbcnet.com/jwks/public.jwks",
  "scopes_supported": ["accounts", "payments", "openid", "fundsconfirmations"],
  "claims_supported": [
    "sub",
    "iss",
    "auth_time",
    "acr",
    "openbanking_intent_id"
  ],
  "response_types_supported": ["code id_token"],
  "grant_types_supported": [
    "authorization_code",
    "client_credentials",
    "refresh_token"
  ],
  "subject_types_supported": ["pairwise"],
  "id_token_signing_alg_values_supported": ["PS256"],
  "request_object_signing_alg_values_supported": ["PS256"],
  "token_endpoint_auth_methods_supported": [
    "private_key_jwt",
    "tls_client_auth"
  ],
  "claims_parameter_supported": true,
  "request_parameter_supported": true,
  "request_uri_parameter_supported": false,
  "token_endpoint_auth_signing_alg_values_supported": ["PS256"],
  "acr_values_supported": ["urn:openbanking:psd2:sca"],
  "tls_client_certificate_bound_access_tokens": true
}

This OpenID provider in question does not implement a CIBA configuration, but a lot of information here relevant to a CIBA client beside what is offered in the revised OAuth Flow object. I'll point to a few (note everywhere I say Client I mean OAuth/OpenID Connect client):

My point is this: By adding OpenID Connect information into OAuth Flow object directly we only scratch the surface on what the Client needs to know in order to understand the OpenID Connect Profile. If we are going to implement CIBA or any other profile directly in the OpenAPI Specification we need to think much more widely about what information the API consumer needs to know to create their implementation, and how a tooling maker can use information effectively. Taking a tooling maker as an example, they would absolutely need to know the information highlighted in the bullets above in order to create a client that could successfully invoke any CIBA implementation currently available across open banking and open finance ecosystems.

There is another important factor here. By furthering this mechanism, where security configuration is represented from within a given OpenAPI description, we make our API descriptions more brittle and more likely to change. Every time OpenID Discovery data changes, the OpenAPI description changes. By having a mechanism where we reference the metadata and provide API consumers a view of how to process that data we make the coupling of OpenID Discovery and the OpenAPI Specification infinitely more flexible and have a means to continually evolve our support for OpenID Connect in all its flavours. There is therefore the potential to a disservice to both the security management lifecycle and API management lifecycle in embedded OpenID Discovery data directly in the OpenAPI Specification.

To 100% clear: My proposal is support CIBA and as many OpenID profiles as we possibly can using the same, holistic mechanism that acts as a bridge between OpenID Discovery and the OpenAPI Specification with the information in the right place, consumed at the right time, with the right tools. I just don't think that embedding it directly in the OpenAPI Specification is the right approach. For me a pointer to the data and the concept of "processing rules" provides that flexibility.

handrews commented 2 months ago

@SensibleWood

I've noticed that the new CIBA fields add a lot to the OpenAPI Flow Object.

@handrews and therein lies the issue.

Ah, yes, my apologies, that was meant to read "add a lot of fields" with the implication being that it has ended up rather imbalanced. I very much agree with you here.

There is another important factor here. By furthering this mechanism, where security configuration is represented from within a given OpenAPI description, we make our API descriptions more brittle and more likely to change.

1000000% yes to this! And everything you said that follows from it.

SensibleWood commented 2 months ago

I was thinking about what I said earlier in this thread about a "rules" based approach, but am a bit stumped on what the right "rules" approach should be.

As an idea I created three rules for CIBA using a custom description syntax, mixing JSON and JSON Path expressions. The idea is predicated on selecting the right properties from the OpenID Discovery data to drive a CIBA Client. If one JSON Path fails to return data the validation fails, and the Client cannot therefore attempt to represent the CIBA Interface to e.g. some trying to test an API call from inside an API client.

So based on this discovery data at the OpenID Provider:

{
    "grant_types": ["urn:openid:params:grant-type:ciba"],
    "backchannel_token_delivery_modes_supported": ["ping", "poll"],
    "backchannel_authentication_endpoint": "https://auth.openapis.org"
}

I used the following JSON Paths to "discover" the data:

{
  "profileName": "CIBA",
  "rules": [
    {
      "property": "grantType",
      "jsonPath": "$.grant_types[?match(@, 'urn:openid:params:grant-type:ciba')]"
    },
    {
      "property": "deliveryMode",
      "jsonPath": "$.[?(@property == 'backchannel_token_delivery_modes_supported')]"
    },
    {
      "property": "authEndpoint",
      "jsonPath": "$.[?(@property == 'backchannel_authentication_endpoint')]"
    }
  ]
}

(Note some JSON Path syntax is as per RFC9535)

If any of the JSON Path expressions return no results, the profile cannot be retrieved and the Client cannot be initialised.

This approach isn't quite the solution (I think). It feels a bit fussy, and not quite what's required to provide a view onto OpenID Discovery data that be used alongside OpenAPI. It is, however, the kind of approach we should be considering to provide the kind of loose-coupling I'm talking about above. JSON Path also features in Overlay and Arazzo, so not completely outside the vernacular of OpenAPI Initiative specifications.

handrews commented 1 week ago

@SensibleWood in the TDC call today we decided that this would make sense as one of the two major focuses of 3.3 (after doing a very quick 3.2 that avoids any big or complicated things).

We also decided that we can add new structures alongside of old ones if the old ones have gotten too complex to be easy to support and maintain (e.g. data modeling involves various combinations of Schema, Media Type, Encoding, Parameter, and Header Objects, three of which have almost-but-not-quite-the-same behavior).

Security configuration is probably the next most complex thing, so if you have ideas for a refactor that might lay some groundwork for Moonwalk, this is a good time to think about that. I've also noticed that the OAuth Flow Object shares some fields with the Parameter Object for yet another data modeling variation, and I'd like to model all things with the same general approach.


With that in mind, I'm trying to figure out what to do with the existing changes for 3.2 as seen in PR 4188, which I posted to migrate them from the old to new 3.2 branching structure.

Obviously the CIBA changes as previously merged should be dropped in favor of this investigation. What about the Device Code Authorization flow? Were you trying to avoid adding more flows in the current system in general, or is this specific to CIBA (please pardon my ignorance about literally everything related to this).

I'm assuming that we can add that deprecated field in 3.2 (even if we end up adding a new mechanism in 3.3), but what about the oauth2MetadataUrl field? Is that its own thing or is it related to what you're talking about here? (I'm unclear on whether this is the same as the openID Connect metadata you mention).

narongdet1995 commented 1 week ago

Subject: Thanks for the Update

Hi,

Thank you for the update. I appreciate you keeping me informed.

Please don't hesitate to reach out if you need anything else from my side or if there's any additional information I can provide.

Looking forward to hearing from you.

Best regards,

https://gpt.space/gmail?source=watermark

SensibleWood commented 1 week ago

@handrews thanks for tagging me

Obviously the CIBA changes as previously merged should be dropped in favor of this investigation. What about the Device Code Authorization flow? Were you trying to avoid adding more flows in the current system in general, or is this specific to CIBA (please pardon my ignorance about literally everything related to this).

☝️ I agree with the assessment on CIBA, as it belongs in the OpenID Connect camp. I think Device Authorization is good as that is still an OAuth 2.0 profile and fits the model (generally speaking)

I'm assuming that we can add that deprecated field in 3.2 (even if we end up adding a new mechanism in 3.3), but what about the oauth2MetadataUrl field? Is that its own thing or is it related to what you're talking about here? (I'm unclear on whether this is the same as the openID Connect metadata you mention).

☝️ I've got no problem with deprecated. oauth2MetadataUrl is good as well, as that leads nicely to a new version of OAuth Flow down the road that "could" look like the OpenID Connect Security Scheme and "just" have an external reference. That all fits nicely into the model I proposed up above for a processing model for OpenID Connect with a bridge between an OpenAPI reference and the metadata/discovery document itself.

handrews commented 1 week ago

@SensibleWood thanks! I will drop the CIBA commit but port the others, and we (by which I mean you and other security-minded folks) can keep working on a bigger re-think for 3.3. BTW, if it's possible to get the bigger re-think into 3.2 I would have no objections to that, it just feels like its a bigger topic and in #4210 I tried to restrict 3.2 to things that are either already-approved proposals or are really small. The main thing is not to delay 3.2 with larger items, since we'll immediately proceed to 3.3.