Skip to content

Changes to Basic Auth to support external logins#12434

Merged
nikolajlauridsen merged 9 commits intov10/devfrom
v10/bugfix/basic-auth-support-for-umbraco-cloud
Jun 2, 2022
Merged

Changes to Basic Auth to support external logins#12434
nikolajlauridsen merged 9 commits intov10/devfrom
v10/bugfix/basic-auth-support-for-umbraco-cloud

Conversation

@bergmania
Copy link
Copy Markdown
Member

@bergmania bergmania commented May 19, 2022

Description

Today the basic auth requies login using the user/password in the umbraco DB. This do not work when using external logins like Google, Twitter or even UmbracoId.

This PR introduces new config settings.

  • Umbraco:CMS:BasicAuth:RedirectToLoginPage
    • Allows you to redirect to the login page instead of prompting for login in browser pop-up.
  • Umbraco:CMS:BasicAuth:SharedSecret:HeaderName
    • A header name you can specify to bypass the requirement about being signed into backoffice.
    • Default value: X-Authentication-Shared-Secret
  • Umbraco:CMS:BasicAuth:SharedSecret:Value
    • A value of the header with the name Umbraco:CMS:BasicAuth:SharedSecret:HeaderName that has to be equal to bypass.

Test

  • Test that the existing Basic auth works as expected
  • Test that the new Umbraco:CMS:BasicAuth:RedirectToLoginPage setting works and redirects to the login page, and when logging in, redirects to the original requested page.
  • Test that the BasicAuth are bypassed when passing correct header + value.
  • Test that the BasicAuth cannot be bypassed when passing incorrect header + value or correct header with empty value

Behavioral Breaking Changes

  • The returnPath when redirected to the login page now includes the entire path and not just the path inside the backoffice SPA

@nikolajlauridsen
Copy link
Copy Markdown
Contributor

nikolajlauridsen commented May 23, 2022

This is very weird, but with this branch, I can't link to external login providers, or at least the microsoft one.

When I try in the backoffice I get redirected to the Microsoft login correctly, but after I log in I get redirected to an error page:

image

If I debug and look at the value in the view data it just says: "An error occurred, could not get external login info", not much help there.

This error seems to persist until I clear my cookies

Looking at the code I'm not sure why this would be though? However this still seems to work fine on the dev branch

@bergmania
Copy link
Copy Markdown
Member Author

Hm.. Maybe some endpoints that needs to be whitelisted? I will investigate

bergmania added 2 commits May 30, 2022 14:35
…uth-support-for-umbraco-cloud

# Conflicts:
#	src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs
@nikolajlauridsen
Copy link
Copy Markdown
Contributor

nikolajlauridsen commented May 31, 2022

Just checked again and now I can link to, and use, external login providers-

It showed another minor issue though. If you log in with email and password, you'll get redirected back to the page you requested, however, if I log in with an external login provider I'll always get redirected to the backoffice.

@simonech
Copy link
Copy Markdown
Contributor

simonech commented Jun 1, 2022

Just a quick heads-up that the doc on our.umbraco already contains these changes:
https://our.umbraco.com/documentation/Reference/V9-Config/BasicAuthSettings/

@simonech
Copy link
Copy Markdown
Contributor

simonech commented Jun 1, 2022

If I may add something to the plate, I don't think this will work if the "backoffice endpoints" are not enabled, which is probably common scenario if you install umbraco in frontend nodes that don't need the backend enabled.

@bergmania
Copy link
Copy Markdown
Member Author

Just a quick heads-up that the doc on our.umbraco already contains these changes:
https://our.umbraco.com/documentation/Reference/V9-Config/BasicAuthSettings/

Thanks, I will talk with the docs team

If I may add something to the plate, I don't think this will work if the "backoffice endpoints" are not enabled, which is probably common scenario if you install umbraco in frontend nodes that don't need the backend enabled.

You are most likely correct, but I guess it depends on the setup (E.g. if you load balance to different domains or just on /umbraco).

Do you have suggestions to make it work? I would hate to add one more allow list. But maybe by allowing redirects to absolute paths if this path has origin equal to UmbracoApplicationUrl?

Copy link
Copy Markdown
Contributor

@nikolajlauridsen nikolajlauridsen left a comment

Choose a reason for hiding this comment

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

Looks good and tests out good for me now 👍

@nikolajlauridsen nikolajlauridsen merged commit faf06be into v10/dev Jun 2, 2022
@nikolajlauridsen nikolajlauridsen deleted the v10/bugfix/basic-auth-support-for-umbraco-cloud branch June 2, 2022 10:19
nikolajlauridsen pushed a commit that referenced this pull request Jun 2, 2022
* Fixed issues with basic auth middleware to support Umbraco Cloud usecase

* Fix redirects to return url, now allows website urls

* Strip potential domain part of returnPath

* Fix default value in appsettings schema

* Reintroduce check of basic auth enabled.

* Fix wrong negation introduced in #12349

* Fixed issues with redirects

* Also check external login cookie, while authenticating backoffice
@simonech
Copy link
Copy Markdown
Contributor

simonech commented Jun 2, 2022

@bergmania
Our scenario is:

  • multiple Frontend nodes in LB, configured without backend endpoints, on one host name
  • Backend node (just one), with backend endpoint enabled on another host name

So, redirection to the login screen on the backend domain might solve the problem, but the auth cookie normally is issued only on the domain on which it is issued. So we should also change the "validity" of the cookie to both domains, but not sure it's possible. If both are subdomains of the same domain it might enough to allow the cookie to valid on all subdomains. But might cause some security issues.

Alternatively, the login endpoint could be "extracted" from the usebackendendpoints and allowed individually. Like it's done for the upgrade and installation endpoints.

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.

4 participants