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

add the state parameter to the oidc authentication request #3466

Merged

Conversation

mortbauer
Copy link
Contributor

Related Issues

@CLAassistant
Copy link

CLAassistant commented Dec 2, 2021

CLA assistant check
All committers have signed the CLA.

@jesmrec jesmrec linked an issue Dec 3, 2021 that may be closed by this pull request
@@ -445,13 +448,15 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
val customTabsBuilder: CustomTabsIntent.Builder = CustomTabsIntent.Builder()
val customTabsIntent: CustomTabsIntent = customTabsBuilder.build()

this.oidcState = UUID.randomUUID().toString().substring(0,15)
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it better to use base64 encoded bytes from SecureRandom here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

definitely better, created a new function in OauthUtils for that

@jesmrec jesmrec closed this Dec 14, 2021
@jesmrec jesmrec reopened this Dec 14, 2021
Copy link
Contributor

@abelgardep abelgardep left a comment

Choose a reason for hiding this comment

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

Just a tiny change and we are ready to go @mortbauer

BTW, rebase this branch with master when you can 👍

@mortbauer mortbauer force-pushed the feature/oidc-with-state-parameter branch from 690ab7b to b826444 Compare December 15, 2021 20:11
Copy link
Contributor

@abelgardep abelgardep left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!!

LGTM, @jesmrec ready to QA

@sonarcloud
Copy link

sonarcloud bot commented Dec 16, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@jesmrec
Copy link
Collaborator

jesmrec commented Dec 17, 2021

QA checks, regression from current authentication methods

  • OIDC + DCR (test instance by jw)
  • OIDC without DCR (ocis.owncloud.works)
  • OAuth2 session creation (regression)
  • OAuth2 token renewal (regression)
  • Basic auth (regression)

Approved from my side

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

Successfully merging this pull request may close these issues.

include state parameter in oidc call to prevent CSRF
5 participants