Skip to content

add support of OAUTHBEARER#79

Merged
schengawegga merged 2 commits intopear:masterfrom
EdouardVanbelle:feat/oauthbearer
Feb 9, 2024
Merged

add support of OAUTHBEARER#79
schengawegga merged 2 commits intopear:masterfrom
EdouardVanbelle:feat/oauthbearer

Conversation

@EdouardVanbelle
Copy link
Contributor

Hello please find the implementation of the OAUTHBEARER authentification
(https://www.rfc-editor.org/rfc/rfc7628.html)

@schengawegga
Copy link
Contributor

Thank you. I am very busy at the moment. I will have a look next week.

Signed-off-by: Edouard Vanbelle <edouard@vanbelle.fr>
@EdouardVanbelle
Copy link
Contributor Author

Hello @schengawegga, no worries. Let me know if you need explanations

@Neustradamus

This comment was marked as off-topic.

@Neustradamus

This comment was marked as off-topic.

@EdouardVanbelle
Copy link
Contributor Author

Hello @schengawegga I added a new commit to this PR: when specifying the fake 'OAUTH' method,
library will elect the best method between 'XOAUTH2' and 'OAUTHBEARER'

Happy new year !

@schengawegga
Copy link
Contributor

Hey @EdouardVanbelle ,

thanks for your contribution. I will review and test this soon. But maybe not in this year, because my family wants some time with me, too ;-)

I wish you a happy new year.

@EdouardVanbelle
Copy link
Contributor Author

EdouardVanbelle commented Jan 6, 2024

Hello @Neustradamus I do not work anymore in OVHCloud
By the way you can contact me via edouard at vanbelle dot fr

@Neustradamus

This comment was marked as off-topic.

@@ -1112,17 +1124,51 @@ protected function authGSSAPI($uid, $pwd, $authz = '')
public function authXOAuth2($uid, $token, $authz, $conn)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All auth* methods are protected, so this one should too, I suppose.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not touch on previous methods already in place, I can correct it to move it into protected, no clue if it is already used outside of this library, I don't think so

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I corrected it in favor of a protected method
@schengawegga let me know your preferences

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference is like @alecpl, to protect all auth* methods, to be straight on this kind of methods.
But, i as you said @EdouardVanbelle, if anybody uses this method from outside, it will break backwards compatibility, and have to be released on a new major version. I also do not think, that anybody uses this from outside, but it is possible.

@alecpl @jparise what do you think about that? can we do a bugfix to change authXOAuth2 method to protected without releasing this under a new major version?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While waiting any feedback I kept the original signature and added a FIXME in comment

@EdouardVanbelle
Copy link
Contributor Author

EdouardVanbelle commented Jan 15, 2024

Thank you @alecpl for your review !

@schengawegga, I hope you will find the time to review this small change ;)

   * OAUTH pseudo method will elect either XOAUTH2 or OAUTHBEARER according to server's capabilities

Signed-off-by: Edouard Vanbelle <edouard@vanbelle.fr>
@EdouardVanbelle
Copy link
Contributor Author

EdouardVanbelle commented Feb 3, 2024

Hello @schengawegga, apologies for the delay, busy time...
I applied your suggestions

@schengawegga schengawegga merged commit 57c969e into pear:master Feb 9, 2024
@schengawegga schengawegga added this to the 1.12.0 milestone Feb 9, 2024
@Neustradamus

This comment was marked as off-topic.

@schengawegga
Copy link
Contributor

Thank you very much @EdouardVanbelle @alecpl
I will do a release of 1.12.0 in the next days.

@EdouardVanbelle
Copy link
Contributor Author

Hello @schengawegga do you know when you can arrange time to release your new version ? :)

@schengawegga
Copy link
Contributor

@EdouardVanbelle Version 1.12.0 released

@Neustradamus

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants