-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add support of OAUTHBEARER #79
Conversation
Thank you. I am very busy at the moment. I will have a look next week. |
Signed-off-by: Edouard Vanbelle <[email protected]>
b8319e1
to
8a06d24
Compare
Hello @schengawegga, no worries. Let me know if you need explanations |
@schengawegga: It has been merged in Roundcube! Now, I hope that SCRAM will be added in Roundcube soon! |
Hello @schengawegga I added a new commit to this PR: when specifying the fake 'OAUTH' method, Happy new year ! |
6451805
to
7ea6681
Compare
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. |
Hello @Neustradamus I do not work anymore in OVHCloud |
@EdouardVanbelle: Thanks, I will do it, you can remove your comment :) |
@@ -1112,17 +1124,51 @@ protected function authGSSAPI($uid, $pwd, $authz = '') | |||
public function authXOAuth2($uid, $token, $authz, $conn) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
7ea6681
to
e822140
Compare
Thank you @alecpl for your review ! @schengawegga, I hope you will find the time to review this small change ;) |
e822140
to
598e46d
Compare
* OAUTH pseudo method will elect either XOAUTH2 or OAUTHBEARER according to server's capabilities Signed-off-by: Edouard Vanbelle <[email protected]>
598e46d
to
ca1be38
Compare
Hello @schengawegga, apologies for the delay, busy time... |
@EdouardVanbelle: Good job! If you can help @alecpl to have SCRAM supports into Roundcube, it will be nice... |
Thank you very much @EdouardVanbelle @alecpl |
Hello @schengawegga do you know when you can arrange time to release your new version ? :) |
@EdouardVanbelle Version 1.12.0 released |
@schengawegga: Thanks, good job too! |
Hello please find the implementation of the
OAUTHBEARER
authentification(https://www.rfc-editor.org/rfc/rfc7628.html)