camaraproject / IdentityAndConsentManagement

Repository to describe, develop, document and test the Identity And Consent Management for CAMARA APIs
Apache License 2.0
18 stars 30 forks source link

Add CAMARA OIDC Profile #113

Closed garciasolero closed 1 month ago

garciasolero commented 5 months ago

What type of PR is this?

What this PR does / why we need it:

Add first draft of CAMARA OIDC Profile:

Which issue(s) this PR fixes:

Special notes for reviewers:

Pending topics to be discussed for inclusion in future versions of the profile:

Changelog input

N/A

Additional documentation

N/A

mhfoo commented 5 months ago

Related to the following: IdentityAndConsentManagement Issue 104 Open-Gateway-Documents Issue 97

bigludo7 commented 5 months ago

Thanks @garciasolero for this contribution. May I ask you to add my colleague @sebdewet in the reviewer list (I cannot do it). Thanks

jpengar commented 5 months ago

Thanks @garciasolero for this contribution. May I ask you to add my colleague @sebdewet in the reviewer list (I cannot do it). Thanks

done.

nickvenezia commented 5 months ago

Is this a Developer Profile being created? PII should be addressed if this is a profile for consumer.

mengan commented 5 months ago

From Mike@t-mobile usa.

i have several comments on the suggested oidc profile for camara.

1) I would like to call out that some carrier will/may require some extensions like DPOP. in which case your non normative examples would be great to include both a Dpop on the token request and a cnf binding to a key in the access_token issued. (though your references are for the id_token the access tokens themselves may (will for us) be in the form of a jwt also.

2) T-mobile us has done a lot of work in dpop bindings to client instances. (a third party may have a client Server in control of the registered client keys. but there may be hundreds/thousands/millions of instances of a client (as a SPA web page, or Device present app. ) in which case the Client server (which creates the client assertion) wishes the Token by bound to a client instance key. (so the client instance can use the issued access token for various service api calls to the carrier). Hence why calling out an example in the doc would be helpfull. client server makes the token call with a refer_binding in the client assertion to the client instance, OR the client server hands the client_assertion to the client instance who can dPOP sign the token request itself.

3) I notice the CIBA flow did not call out Scope or other OIDC elements like nonce as required or optional in the Post to initiate the request. From our (tmo us) experience, these back channel requests should contain ALL attributes of the oidc request, especially since an oidc id_token is suggested as a result.

4) for the CIBA back channel flow. T-Mobile experience has shown a need for a user facing context (UI MESSAGING) need. otherwise users may not have enough information to understand what is being asked of them. this is an important ani phishing element as well. examples "to enable the xxx app/purchase". at which point the carrier may then dynamically insert copy appropriate to the scopes being requested. (company xyz is asking for access to your device location, Quality of service, make and receive calls and messages, ..... for your number). Note: an alternative approach is that when a company/client is defined attributes of that client id definition define this user facing copy.

5) ciba ttl... through experience we (tmo us) have found two different default experiences for ciba. the first what we might call User aware flows could be for when a user is on the phone to a call center, or visiting a retail location.. the user is aware of a transaction and can quickly look at their phone. For these kinds of flows we see a ttl of 5-10 minutes to be appropriate. The second kind of expereince is where the user is not aware of the transaction, this can even include cases where the user is not even awake or might not have their phone in their hands and charged... these kinds of flows a ttl in the 8-12 hours is typical. therefore tmo has in the past let clients pass a ttl (number of seconds) as an attribute in the ciba call flow.

6) your Client credential flow specifically calls out that SCOPES are not needed.. i think this might be a mistake. we believe there will be examples where a client uses user facing (oidc code flow, or ciba flow) for the FIRST interaction with a user. but then once that user has provided the consents... the client could continue (without needing to make refresh token juggling) to simply make a request for a client credential token and request the scope in that token... Once that token hits the resource service. the service can verify both (1) the client is allowed to hit this service because idp confirmed they have scope for this... (2) the User the client is trying to hit with this service as a body attribute to the resource api call... has a recorded consent for this scope to this client. Note: in addition to DIRECT user interaction to collect scope consents, T-Mobile usa is also looking at other ownership flows.... Ie... if Company xyz has a billing account with thousands of phone lines, that company Could create a client and at client provisioning time we can record these "consents". the client could make an api call to any of the thousand phone numbers and because this client has an exisitng consent record (via an ownership role) the transactions would be processed. on the other hand if the client tried querying a different number the request may fail with 403 as no consent (via explicit user consent, or ownership role) exists.

hmm... overall its a great first stab at getting everyone on the same page for oidc elements for all carriers to support.

Thanks.

AxelNennker commented 5 months ago

During the meeting there was the argument that a repeat-everything profile would be easier to read for developers. I do not think that argument is relevant because nobody sane is going to start developing e.g. an AZ from scratch. OIDF recommends to start from scratch but to use existing libraries. https://openid.net/developers/certified-openid-connect-implementations/

Most of the telcos either already have an OIDC implementation and if not we pick a vendor who has some proven track record implementing OIDC and OAuth2 SW.

So, I think that this policy should list security requirements that improve the security of OIDC. And this policy should reduce the number of features an implementer has to implement fostering interoperability. Example: if we require PKCE to be used in authorization code flow as an Anti-CSRF then developers know that other measures to counter CSRF are not needed. Example: if we require sender-constraint access tokens with DPoP then developers know what to do.

If this profiles repeats all the standards it profiles than this document gets quite long, which is then unreadable for developers. So, I think that "readability" is not an argument that holds water.

Elisabeth-Ericsson commented 5 months ago

@all: First of all I want to give a big appreciation to the colleagues from Telefonica for putting this great document together. In my view we should find a way to properly review it and get it into the best possible form. This will be the central document used by all developers and thus requires detailed review from all participants. I think that we need time for that.

Given all the discussions in the team, I read the agreement documented in the technical steering group about the release content. (2024-01-18 TSC Minutes - CAMARA Project - Confluence) extracted from the meeting minutes is the following: _1. V0.1 to be generated. All issues marked as required for the release are solved (Intended date for release will be next Jan 22 - Jan 26.)

  1. V0.2 to be progressed as fast as possible. Scope: clarifications on existing scope requested by existing issues
  2. V0.3 will include enhancements_ OIDC profile: To be created in CAMARA. Not to rely on starting it in OIDF working group to avoid risk of delay.

The agreement does not state the release, the OIDC profile document should be put in. In order to buy us some discussion time, would it be possible to include the most needed clarifications on the short term agreement in the already existing document (https://github.com/camaraproject/IdentityAndConsentManagement/blob/main/documentation/CAMARA-API-access-and-user-consent.md) as 0,2 - and only concentrate on the most urgent problems only ? In parallel we would safe time and get some room for discussions on the new document for the OIDC profile? Then we would use the new document on OIDC profile (content of this pull request) to be the embryo of 0,3. I acknowledge that the ODIC profile document is very much needed. But is it reasonable to spend so much effort in this (very good) document and have it focused on the short term agreement (which is not really future proof) ?

I'm a bit afraid that this new document will become the bible. As soon as the implementations will take off now, gain speed and then - with this short term agreement - we cement it and build a burden into our solution we cannot get rid of any more.

jpengar commented 5 months ago

During the meeting there was the argument that a repeat-everything profile would be easier to read for developers. I do not think that argument is relevant because nobody sane is going to start developing e.g. an AZ from scratch. OIDF recommends to start from scratch but to use existing libraries. https://openid.net/developers/certified-openid-connect-implementations/

Most of the telcos either already have an OIDC implementation and if not we pick a vendor who has some proven track record implementing OIDC and OAuth2 SW.

So, I think that this policy should list security requirements that improve the security of OIDC. And this policy should reduce the number of features an implementer has to implement fostering interoperability. Example: if we require PKCE to be used in authorization code flow as an Anti-CSRF then developers know that other measures to counter CSRF are not needed. Example: if we require sender-constraint access tokens with DPoP then developers know what to do.

If this profiles repeats all the standards it profiles than this document gets quite long, which is then unreadable for developers. So, I think that "readability" is not an argument that holds water.

I understand your point, and you presented it correctly in yesterday's call, but after asking the WG participants and considering the different opinions presented during the meeting, the way forward was agreed to go with the self-contained format + compromise to properly note any changes related to the reused standard.

jpengar commented 5 months ago

@Elisabeth-Ericsson Regarding your comment https://github.com/camaraproject/IdentityAndConsentManagement/pull/113#issuecomment-1921382449, please see Diego's comment here -> https://github.com/camaraproject/IdentityAndConsentManagement/pull/113#discussion_r1474604755

And regarding your concerns, the existing purpose agreement is what's going to be implemented in the short term anyway. It is not more or less valid/official because we documented it in CAMARA-API-access-and-user-consent.md and/or the OIDC profile. And as I said yesterday, the OIDC profile is not only about the purpose solution, but also about including other implementation details that have already been requested as needed by other participants in their current implementations (client authentication, CIBA login_hint, error scenarios, etc.). That's why the profile is proposed for the v0.2.0 release. And in fact, in the latest updates of the profile in this PR, the purpose solution is just a reference to API-access-and-user-consent.md.

AxelNennker commented 5 months ago

During the meeting there was the argument that a repeat-everything profile would be easier to read for developers. I do not think that argument is relevant because nobody sane is going to start developing e.g. an AZ from scratch. OIDF recommends NOT (corrected) to start from scratch but to use existing libraries. https://openid.net/developers/certified-openid-connect-implementations/ Most of the telcos either already have an OIDC implementation and if not we pick a vendor who has some proven track record implementing OIDC and OAuth2 SW. So, I think that this policy should list security requirements that improve the security of OIDC. And this policy should reduce the number of features an implementer has to implement fostering interoperability. Example: if we require PKCE to be used in authorization code flow as an Anti-CSRF then developers know that other measures to counter CSRF are not needed. Example: if we require sender-constraint access tokens with DPoP then developers know what to do. If this profiles repeats all the standards it profiles than this document gets quite long, which is then unreadable for developers. So, I think that "readability" is not an argument that holds water.

I understand your point, and you presented it correctly in yesterday's call, but after asking the WG participants and considering the different opinions presented during the meeting, the way forward was agreed to go with the self-contained format + compromise to properly note any changes related to the reused standard.

Yesterday I presented a different point. This point is about the "readablity"-argument, which yesterday I had no counter argument to, but today I have the above and I think that the readability argument is wrong.

I think the changes-only policy document is much easier to read.

Usually, in this working group we discuss arguments for a few days and give participants time to review and comment.

See the redirect_uri example: Today a change was merged, that I think is wrong and it also does not mark the definition as changed from the standard.

jpengar commented 5 months ago

Yesterday I presented a different point. This point is about the "readablity"-argument, which yesterday I had no counter argument to, but today I have the above and I think that the readability argument is wrong.

I think the changes-only policy document is much easier to read.

Usually, in this working group we discuss arguments for a few days and give participants time to review and comment.

See the redirect_uri example: Today a change was merged, that I think is wrong and it also does not mark the definition as changed from the standard.

There have been days of feedback on the profile format since the PR was created, there have been different opinions, and yesterday's meeting was precisely to agree on a way forward and unblock this hot topic.

If you've thought about it a second time and now you've come up with some more analysis, that's fine. But yesterday we did agree on a way forward. So I understand that you are now proposing to reopen the discussion??

If the rest of the participants feel it's necessary, I would disagree, but it would be the WG decision. And in that case we can give some time for feedback as we always do. But this has a big impact on the PR, and we were already working and updating the PR according to yesterday's agreement. It is an inconvenience.

AxelNennker commented 5 months ago

I created https://github.com/AxelNennker/IdentityAndConsentManagement/blob/camara_oidc_profile/documentation/CAMARA-Security-Interoperability.md

Much shorter much easier to read, I think. Hopefully I captured everything that CAMARA-OIDC-Profile wants to say about usage of OIDC, CIBA, etc

hdamker commented 5 months ago

@garciasolero @jpengar @Elisabeth-Ericsson @shilpa-padgaonkar @eric-murray

First of all thanks to @garciasolero for the initial draft of this document. I understand that it is written with two different audiences in mind

The target audience for this document is the service/technical departments of operators exposing network functions via standard CAMARA APIs. And applications or client systems that consume CAMARA standard APIs to make use of the operator's network capabilities.

But despite the great effort I think that it is unfortunately missing both audiences with the attempt to be "self contained".

Let me start with "service/technical departments of operators exposing network functions via standard CAMARA APIs". They would rely on the referred standards and need to know (only) the points which CAMARA is defining in addition to ensure interoperability and security. For them the document has too much duplicated information which is already defined with in the referenced standards. They would need exactly the information which is provided within the short profile document proposed by @AxelNennker. For the exact definition of the expected behaviour the document in this PR has a high risk to get an "alternative standards" to OIDC, but without being ever complete (e.g. the complete definition of terminology is missing).

On the other side are the "applications or client systems that consume CAMARA standard APIs". Actually the audience are here the developers of the these applications or client systems. For them indeed a self-contained document which explains what they need to initiate the flows, authenticate, what the expect in the tokens, explanations of potential errors, etc would be helpful. We should not require them to read multiple documents (also not within CAMARA) and to dig into standards which are written for implementators. But that should be a documentation from a developer perspective, not mixed up with design decision how to implement the OIDC standards. Reading over the current document I don't have the impression that the current PR would fulfil the need of this audience.

Said this my recommendation would be to reduce the profile document in this PR to just the differences to the referred standards and consider to write an additional document which describes with examples, flow diagrams etc the use of CAMARA APIs from developers perspective, referring to the standards for further details, including mentioning the differences / limitations imposed by the CAMARA profile to ensure interoperability. This approach would ensure that we can fix in short time the necessary agreements for implementations and then in parallel to implementations work on proper documentation for developers (the main target audience of CAMARA APIs).

hdamker commented 4 months ago

@diegogonmar asked that I put my following suggestion from a now closed discussion into a separate thread comment so we collect group opinion on it (origin: https://github.com/camaraproject/IdentityAndConsentManagement/pull/113#discussion_r1478838189):

As a comprise I would propose that the profile can be already part of v0.2, if in time agreed, and if it does not codify the short-comings of the short-term solution. Which includes that it also not refer to (https://github.com/camaraproject/IdentityAndConsentManagement/blob/release-0.2.0/documentation/CAMARA-API-access-and-user-consent.md#applying-purpose-concept-in-the-authorization-request) but only the other way round (the API-access-and-user-consent.md refers for additional clarifications to the profile).

I would like to see it together with my proposal within the previous comment, to focus first on a short profile document to get the necessary clarifications agreed, and doing a comprehensive developer documentation afterwards based on the profile.

davidgtonge commented 4 months ago

Just adding to the discussion, there is quite a lot of repetition here from existing specs. Is it worth considering referencing https://openid.bitbucket.io/fapi/fapi-2_0-security-profile.html

This is a secure profile of OAuth2 that has the following advantages:

  1. extensive conformance tests
  2. extensive vendor support
  3. formal security analysis

Axel's PR looks like it could build on this existing work.

AxelNennker commented 4 months ago

Regarding offline_access, please mark this as a change to the OIDC standard on offline_access. The standard defines that the client gets an access token for the user-info endpoint. While this profile defines that the clients gets a refresh token and and access token for a RS endpoint

garciasolero commented 1 month ago

This self-contained profile pull request is being closed because it has been decided to use another profile format that has been merged into main branch in pull request #121.