[GitHub] [nifi] Eetami commented on pull request #6782: NIFI-10456: Use Basic authentication in nifi-oauth2-provider-services

[GitBox <gi...@apache.org>]

Eetami commented on PR #6782:
URL: https://github.com/apache/nifi/pull/6782#issuecomment-1352667572

   > Thanks for the contribution @Eetami!
   > 
   > Supporting Basic Authentication makes sense given the RFC documentation. However, the change needs to be implemented in a way that maintains compatibility with the current implementation.
   > 
   > Introducing a new configuration property named something like `Authentication Strategy`, with initial options including `Request Parameters` and `Basic Authentication` would provide a way to introduce support without breaking existing implementations.
   > 
   > One other note, `OAuth2TokenProviderImpl` is deprecated, so it does not need to be changed.
   
   Hey, thanks for your feedback! Indeed the implementation is not _technically_ backwards compatible, BUT assuming that all auth servers support Basic authentication (as mandated by the RFC), then this should not be a breaking change. Of course if you worry that there might be some auth servers that do not support Basic authentication or they only support some other form of client authentication, then we should probably implement that using the aforementioned `Authentication Strategy` option. Though I feel like this implementation is most aligned with the RFC and should, by all accounts, be the default strategy.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org