Skip to content

Complete apache auth (SAML/SSO) flow with redirect page#40161

Merged
phil-davis merged 2 commits intomasterfrom
e5225-bugfix/apache-login-app-loading-issues
Jul 13, 2022
Merged

Complete apache auth (SAML/SSO) flow with redirect page#40161
phil-davis merged 2 commits intomasterfrom
e5225-bugfix/apache-login-app-loading-issues

Conversation

@mrow4a
Copy link
Contributor

@mrow4a mrow4a commented Jun 21, 2022

Description

Without this change, server-side redirect to default page for Apache Auth (e.g. Shibboleth) resulted in session data missing on client side. Flow was incomplete and was missing final redirect page on client side to correctly handle redirect_url and persist the session data on the client.

Related Issue

How Has This Been Tested?

  • manually in internal oc2.lab test instance

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • Base code changes
  • Make sure to handle corner cases in login
  • Unit tests added
  • Acceptance tests added
  • Changelog item, see TEMPLATE

@mrow4a mrow4a requested a review from DeepDiver1975 June 21, 2022 20:56
@mrow4a mrow4a self-assigned this Jun 21, 2022
@update-docs
Copy link

update-docs bot commented Jun 21, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@mrow4a mrow4a changed the title complete apache auth flow with redirect page WIP: complete apache auth flow with redirect page Jun 21, 2022
@mrow4a mrow4a force-pushed the e5225-bugfix/apache-login-app-loading-issues branch 3 times, most recently from 999e055 to d27da44 Compare June 23, 2022 19:43
@mrow4a mrow4a changed the title WIP: complete apache auth flow with redirect page complete apache auth flow with redirect page Jun 25, 2022
@mrow4a mrow4a force-pushed the e5225-bugfix/apache-login-app-loading-issues branch from d27da44 to 90f4a67 Compare June 25, 2022 13:37
@mrow4a mrow4a changed the title complete apache auth flow with redirect page Complete apache auth (SAML/SSO) flow with redirect page Jun 25, 2022
@mrow4a mrow4a force-pushed the e5225-bugfix/apache-login-app-loading-issues branch from 90f4a67 to 0b6a04b Compare June 25, 2022 13:42
@mrow4a
Copy link
Contributor Author

mrow4a commented Jun 27, 2022

I did not add acceptance tests as part of this PR, as these need to be in the apps handling the backend, in here we can only cover with unit tests

@mmattel
Copy link
Contributor

mmattel commented Jun 30, 2022

@d7oc maybe of interest for you

@mrow4a mrow4a requested a review from AlexAndBear July 10, 2022 16:42
@mrow4a
Copy link
Contributor Author

mrow4a commented Jul 10, 2022

@janackermann @DeepDiver1975 could you have a look here? target for this is oc10.11

@mmattel
Copy link
Contributor

mmattel commented Jul 13, 2022

I just made a small doc fix, unhide to see details.

@ownclouders
Copy link
Contributor

ownclouders commented Jul 13, 2022

💥 Acceptance tests pipeline apiWebdavPreviews-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/36296/97

@DeepDiver1975
Copy link
Member

As discussed offline - as long as this works 👍

@mmattel
Copy link
Contributor

mmattel commented Jul 13, 2022

@phil-davis I just added a comment text fix which resulted in Build was killed ... any CI hickup?

@phil-davis phil-davis force-pushed the e5225-bugfix/apache-login-app-loading-issues branch from 5f48833 to d23f5db Compare July 13, 2022 09:56
@phil-davis
Copy link
Contributor

@phil-davis I just added a comment text fix which resulted in Build was killed ... any CI hickup?

This PR was quite a lot of commits behind master. I rebased just now - let's see if CI is happier.

@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/core/36296/97/14
apiWebdavPreviews/previews.feature:210

That is failing in master today - issue owncloud/QA#750 - it is being looked at...

@phil-davis phil-davis force-pushed the e5225-bugfix/apache-login-app-loading-issues branch from d23f5db to c70ae52 Compare July 13, 2022 11:50
@sonarqubecloud
Copy link

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

75.0% 75.0% Coverage
0.0% 0.0% Duplication

@phil-davis phil-davis merged commit fc34ac6 into master Jul 13, 2022
@sonarqubecloud
Copy link

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

75.0% 75.0% Coverage
0.0% 0.0% Duplication

@phil-davis phil-davis merged commit fc34ac6 into master Jul 13, 2022
@delete-merged-branch delete-merged-branch bot deleted the e5225-bugfix/apache-login-app-loading-issues branch July 13, 2022 12:39
@mrow4a
Copy link
Contributor Author

mrow4a commented Jul 14, 2022

@phil-davis btw, as mentioned in https://github.com/owncloud/enterprise/issues/5225#issuecomment-1167075255 and https://github.com/owncloud/enterprise/issues/5225#issuecomment-1166973577, as there is no acceptance test in user_shibolleth , we would need to make sure all works with SAML/SSO enabled in QA env.

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.

5 participants