Thanks Janak for your detailed review and opening the GitHub issues.

On Wed, Nov 13, 2024 at 9:39 AM Janak Amarasena <[email protected]>
wrote:

> Hi All,
>
> I created GitHub issues[1] #125 to #131 covering the feedback I provided
> through my previous email.
>
> [1] - https://github.com/oauth-wg/oauth-first-party-apps/issues
>
> Best Regards,
> Janak Amarasena
>
> On Fri, Nov 1, 2024 at 11:43 AM Janak Amarasena <[email protected]>
> wrote:
>
>> Hi All,
>>
>> I have gone through the OAuth 2.0 for First Party Applications draft (
>> draft-ietf-oauth-first-party-apps-00) and have some feedback on it. I
>> think this is a much needed standard. At my organization(WSO2) also we have
>> seen a significant demand from customers requesting to do API centric
>> authorization largely due to the need for seamless UX. We have seen places
>> where organizations lean more towards UX disregarding security best
>> practices. Due to the demand we ourselves did an extension for OAuth to
>> solve the same problem this specification is addressing and at the time if
>> this specification existed we would have definitely implemented this
>> without going ahead with our own extension.
>>
>> Please find my feedback below;
>>
>> Under section 5.1. “Authorization Challenge Request” the spec lists the
>> “code_challenge” and the "code_challenge_method" as optional parameters. As
>> this protocol establishes direct communication between the client and the
>> AS I don’t see a real requirement to mention PKCE related parameters
>> here. Please let me know if I have missed anything here.
>>
>> As I understood, using these two parameters in the authorization
>> challenge request is useful only when the client expects that it will have
>> to perform a redirect based authorization flow and also the AS supports PAR
>> capabilities through the authorization_challenge_endpoint. I think this
>> will be an edge case and given the spec mentions it supports all extensions
>> applicable to the authorization endpoint I don’t see a major need to
>> specifically mention these two parameters under this section. I think this
>> could also cause confusion to implementers.
>>
>> Under section 5.2.2.1. “Redirect to Web Error Response” the spec mentions
>>
>>    In this case, the client is expected to initiate a new OAuth
>>
>>    Authorization Code flow with PKCE according to [RFC6749] and
>>
>>    [RFC7636].
>>
>>    If the client expects the frequency of this error response to be
>>
>>    high, the client MAY include a PKCE ([RFC7636]) code_challenge in the
>>
>>    initial authorization challenge request.  This enables the
>>
>>    authorization server to essentially treat the authorization challenge
>>
>>    request as a PAR [RFC9126] request, and return the request_uri and
>>
>>    expires_in as defined by [RFC9126] in the error response.  The client
>>
>>    then uses the request_uri value to build an authorization request as
>>
>>    defined in [RFC9126] Section 4.
>>
>> I think it would be good to add some text to the spec mentioning the
>> possibility to use the auth_session in this new authorization request such
>> that the user can continue the login from where the user left off.
>> Something similar is mentioned in section 6.1. for step-up authentication.
>>
>>
>> I have some concerns with the authorization_challenge_endpoint being able
>> to act as the PAR endpoint. I understand the improved experience gained
>> here but this essentially creates two standard endpoints that can do
>> similar things. Instead it might be possible to use the auth_session to
>> maintain the complete context. However this could be overloading the
>> expectations of the auth_session.
>>
>> Under section 5.3.1. “Auth Session” spec mentions;
>>
>>    The auth_session value is completely opaque to the client, and as
>>
>>    such the authorization server MUST adequately protect the value from
>>
>>    inspection by the client, for example by using a random string or
>>
>>    using a JWE if the authorization server is not maintaining state on
>>
>>    the backend.
>>
>> I think the intention behind mandating to maintain the opaqueness is to
>> protect any sensitive information. Depending on the AS implementation it
>> could decide on using an auth_session value which is not opaque but also
>> does not contain any sensitive data. I think it would be better to
>> recommend that the AS uses adequate measures such as encryption in the
>> event they are using something other than an opaque value that contains
>> sensitive data. The current mandating will put an unnecessary burden on the
>> AS to encrypt and decrypt data if it doesn’t contain sensitive information.
>>
>> In the same section it mentions;
>>
>>   To mitigate the risk of session hijacking, the 'auth_session' MUST be
>>
>>    bound to the device, and the authorization server MUST reject an
>>
>>    'auth_session' if it is presented from a different device than the
>>
>>    one it was bound to.
>>
>> I completely agree on the need to mitigate the risk of session hijacking.
>> However, in the current text, although it's not directly mentioned here, it
>> will require the AS to have a proof of possession mechanism in place as
>> pointed in Section 9 "Security Considerations". This can be a major
>> barrier to implement this specification as the AS should also implement
>> another specification. There can be different ways the implementation can
>> solve this problem without needing proof of possession. For example if the
>> auth_session is only transmitted between the AS and the client then it will
>> be protected with TLS in transit and the client and AS can use independent
>> mechanisms to protect the auth_session at rest. Since this specification
>> applies only to first party applications the implementers will have full
>> control over how the client and the AS protects the data, and therefore can
>> make sure adequate protection is in place for the auth_session. Due to this
>> I think it is better to change wording from mandating to a recommendation.
>>
>> Regarding section “7. Resource Server Error Response” I am wondering
>> whether this section is required at all as this spec makes no addition to
>> RFC9470. I guess this section is there to provide clarity due to RFC9470
>> using the authorization endpoint in its text.
>>
>> Under section 9.5.1. DPoP: Demonstrating Proof-of-Possession it mentions
>> “… The authorization server MUST ensure that the same key is used in all
>> subsequent Authorization Challenge Requests, or in the eventual token
>> request…” I think it was meant to say “... Authorization Challenge
>> Requests, and in the eventual token request…”
>>
>> Best Regards,
>>
>> Janak Amarasena
>>
>> _______________________________________________
> OAuth mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
>
_______________________________________________
OAuth mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to