Skip to content
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

Support for openid connect #7509

Merged
merged 2 commits into from
Nov 20, 2019
Merged

Support for openid connect #7509

merged 2 commits into from
Nov 20, 2019

Conversation

ogoffart
Copy link
Contributor

@ogoffart ogoffart commented Oct 9, 2019

No description provided.

@ogoffart ogoffart requested a review from guruz October 9, 2019 11:51
@ogoffart ogoffart mentioned this pull request Oct 9, 2019
@ogoffart
Copy link
Contributor Author

From drone:

/drone/src/src/gui/creds/oauth.cpp:26:10: fatal error: QRandomGenerator: No such file or directory

QRandomGenerator was added in Qt 5.10. I don't really want to add a fallback with the insecure qrand
So i'd like to make Qt 5.12 a minimum required Qt version for the client 2.7

@michaelstingl
Copy link
Contributor

So i'd like to make Qt 5.12 a minimum required Qt version for the client 2.7

Okay, works for me 👍

@ogoffart
Copy link
Contributor Author

Make Qt 5.12 minimum requirement: #7521
Update to CI: owncloud-ci/client#1

@mmattel
Copy link
Contributor

mmattel commented Nov 15, 2019

@TheOneRing mind review/approving?

src/gui/creds/oauth.cpp Outdated Show resolved Hide resolved
src/gui/creds/oauth.cpp Outdated Show resolved Hide resolved
src/gui/creds/oauth.cpp Show resolved Hide resolved
src/gui/creds/oauth.h Show resolved Hide resolved
src/libsync/creds/httpcredentials.cpp Show resolved Hide resolved
QString accessToken = json["access_token"].toString();
if (jsonParseError.error != QJsonParseError::NoError || json.isEmpty()) {
// Invalid or empty JSON: Network error maybe?
qCWarning(lcHttpCredentials) << "Error while refreshing the token" << reply->errorString() << jsonData << jsonParseError.errorString();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any error state you need to set in some variable or return value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea is that the jobs get re-tried and will then process to the normal authentication method.
Yeah, maybe something better could be done, but we really don't want to disturb the user in this case anyway. This is not new code, it just got one more level of identation.

@ogoffart ogoffart requested a review from guruz November 18, 2019 12:26
@guruz
Copy link
Contributor

guruz commented Nov 20, 2019

@michaelstingl @HanaGemela FYI

@guruz guruz added the ReadyToTest QA, please validate the fix/enhancement label Nov 20, 2019
@guruz guruz merged commit d3e02b9 into master Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ReadyToTest QA, please validate the fix/enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants