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

Improve support for integrating with external identity providers (Auth0, IdentityServer, Azure AD etc) #1623

Merged
merged 16 commits into from
May 7, 2021

Conversation

tedvanderveen
Copy link
Contributor

When using an external (OAuth etc) identity/auth/token provider, Piranha itself does not handle bits like login (username/password). To better support these scenarios I have moved the interface ISecurity and the related Razor pages (Logout and Login) to a separate project. This project can than be added (using the provided extension methods) when needed.

Related to issue #1622

@tidyui
Copy link
Member

tidyui commented May 5, 2021

As I wrote in my comment in #1622 the solution here is to add your own login page in your application which will then override the embedded version. As much as I appreciate the initiative we will not add more complexity to the architecture by adding a package for a single view of the manager interface.

Best regards

Håkan

@tedvanderveen
Copy link
Contributor Author

tedvanderveen commented May 5, 2021

Hi. I do not have my own Login page. We are implementing https://github.com/AzureAD/microsoft-identity-web for our project.

It comes with controllers/login pages OOTB:
https://github.com/AzureAD/microsoft-identity-web/tree/master/src/Microsoft.Identity.Web.UI/Areas/MicrosoftIdentity

I do not see the reason why there should be something so opinionated as a classic login/password credential thing as implemented through ISecurity, as a part of the root core implementation of Piranha? IMHO anything related to auth should be optionally plugged in.

Anyway, can you please provide guidance on how to incorporate https://www.nuget.org/packages/Microsoft.Identity.Web.ui into a Piranha deployment?

@tedvanderveen
Copy link
Contributor Author

@tidyui I removed the new project / package. And moved the ISecurity interface, along woth the Login and Logout pages, to the place where those are actually used: Piranha.AspNetCore.Identity. So users that want to use Microsoft.AspNetCore.Identity functionality keep same experience without any breaking changes. But any deployment out there that wants to deploy an external identity/token service (Auth0, IdentityServer, Azure AD, etc), does not have to deal with ISecurity fanfare nor implementing custom Login pages.

@tidyui
Copy link
Member

tidyui commented May 6, 2021

Looking at the AccountController in the project you referenced it looks like it will be listening to the following routes:

  • GET /<area>/account/signin/{scheme}
  • GET /<area>/account/challenge/{scheme}
  • GET /<area>/account/signout/{scheme}
  • GET /<area>/account/resetpassword/{scheme}
  • GET /<area>/account/editprofile/{scheme}

Where the included manager login listens to the following routes:

  • GET /manager/login
  • POST /manager/login
  • GET /manager/logout

None of these routes seem to conflict with each other. If you simply want user to use another external login and logout service, wouldn't the simplest solution just be to add a middleware component first in the pipeline that just redirects any requests that comes to /manager/login and /manager/logout to the adequate URL:s?

@tidyui
Copy link
Member

tidyui commented May 6, 2021

Some other thoughts on this. Yes, we could move the login/logout pages to a different project as long as we decide on how to handle a couple of things in the manager.

  1. When the user clicks "logout" in the UI he/she needs to be sent somewhere. As long as this is a GET request we can make this configurable.
  2. When the user is signed out due to lack of activity, i.e the cookie expires the manager needs to know where to send the user to re-authenticate. As long as this is a GET request we can make this configurable as well.

The ISecurity interface can't be moved to Piranha.AspNetCore.Identity as this is just one implementation of local security. For example, Piranha.AspNetCore.SimpleSecurity has nothing to do with ASP.NET Identity and shouldn't have a reference to it.

If it's needed to extract this from the manager (if you don't use the built in login/logout and instead redirect like I mentioned previously there's no need to implement ISecurity as this service is only used by these pages) it probably best to move it to a separate project like you did in the first place. However I'd still like to know what is holding you back from completing your implementation with them just "being around" without being used :)

Anything else I'm missing here?

@tedvanderveen
Copy link
Contributor Author

Adding required complexity by implementing custom middleware components and -not to forget- adding (and DI injecting) some bogus implementation of the ISecurity interface to avoid related exceptions (#654, #267, more) is avoidable when moving those "local" login/ISecurity bits out of the core Piranha package and into the optional packages where it actually is used.
This reduces complexity for any real world production deployments that want to leverage existing/external identity stores (Auth0, IdentityServer, Azure AD, etc etc), without introducing breaking API changes for deployments that want to deploy a local identity store via Microsoft.AspNetCore.Identity.

@tedvanderveen tedvanderveen changed the title Add new project Piranha.Manager.Security for encapsulating all bits used for "local" / integrated security Improve support for integrating with external identity providers (Auth0, IdentityServer, Azure AD etc) May 6, 2021
@tidyui
Copy link
Member

tidyui commented May 6, 2021

@tedvanderveen What are your thoughts on my questions (1 & 2) in my previous comment. Would a configurable GET Url be sufficient for your implementation?

@tedvanderveen
Copy link
Contributor Author

@tidyui

Some other thoughts on this. Yes, we could move the login/logout pages to a different project as long as we decide on how to handle a couple of things in the manager.

  1. When the user clicks "logout" in the UI he/she needs to be sent somewhere. As long as this is a GET request we can make this configurable.
  2. When the user is signed out due to lack of activity, i.e the cookie expires the manager needs to know where to send the user to re-authenticate. As long as this is a GET request we can make this configurable as well.

The ISecurity interface can't be moved to Piranha.AspNetCore.Identity as this is just one implementation of local security. For example, Piranha.AspNetCore.SimpleSecurity has nothing to do with ASP.NET Identity and shouldn't have a reference to it.

If it's needed to extract this from the manager (if you don't use the built in login/logout and instead redirect like I mentioned previously there's no need to implement ISecurity as this service is only used by these pages) it probably best to move it to a separate project like you did in the first place. However I'd still like to know what is holding you back from completing your implementation with them just "being around" without being used :)

Anything else I'm missing here?

In response to your questions

  1. Indeed, a Logout button/menu item is the only UI element required when integrating an external identity service. When integrating with Microsoft.Identity.Web and Microsoft.Identity.Web.UI, the Log out link should point to Signout() action of the AccountController. Maybe we can extend the PiranhaRouteConfig class for this?
  2. This should be handled automatically by the external identity service implementation package/code. The Manager only specifies [Authorize(Policy = Permission.Admin)] required policies, the rest is handled for us.

@tedvanderveen
Copy link
Contributor Author

In all, I guess the main objective of this effort is to remove anything related to user account credentials (Login UI bits and anything related to the concept of a "password") out of the core Piranha package.

@tidyui
Copy link
Member

tidyui commented May 6, 2021

@tedvanderveen I took your branch and pushed it here (https://github.com/PiranhaCMS/piranha.core/tree/wortell-feature/isolate-local-security/core/Piranha.Manager.LocalAuth), could you take a look at it. I have:

@tedvanderveen
Copy link
Contributor Author

@tedvanderveen I took your branch and pushed it here (https://github.com/PiranhaCMS/piranha.core/tree/wortell-feature/isolate-local-security/core/Piranha.Manager.LocalAuth), could you take a look at it. I have:

@tidyui Great stuff! Running couple of tests now.

@tidyui
Copy link
Member

tidyui commented May 6, 2021

@tedvanderveen When you get your application up and running, it would be more than awesome if you would contribute an article about it to our docs repo (https://github.com/PiranhaCMS/piranha.core.docs). This is then automatically published to the "Docs" section on our official site.

Best regards

@tedvanderveen
Copy link
Contributor Author

@tidyui got it all tested and working over here. 💹 (using Azure Active Directory B2C, by the way).

@tidyui tidyui merged commit 63e7f6d into PiranhaCMS:master May 7, 2021
@tidyui
Copy link
Member

tidyui commented May 7, 2021

Merged into master with my changes! Will be released on Nuget in version 9.1

@tidyui tidyui added this to the Version 9.1 milestone May 9, 2021
@jensbrak
Copy link
Contributor

This is exciting news! I've been longing to try to integrate with external login. This seems to take things several steps further in that direction. Great job!

@AleksandrAlbert
Copy link

@tidyui got it all tested and working over here. 💹 (using Azure Active Directory B2C, by the way).

Hey, could you post a gist of what your Startup looks like to get this working? I am trying to use this with Microsoft Identity though they should be similar.

@tidyui
Copy link
Member

tidyui commented Jul 29, 2021

@AleksandrAlbert I suggest reaching out to @tedvanderveen who was responsible for the change and is using Piranha with external providers. Personally I only use ASP.NET Identity and local authentication!

@tedvanderveen
Copy link
Contributor Author

@AleksandrAlbert Using Nuget package Microsoft.Identity.Web.UI I have the following setup in startup.cs:

            services
                .AddAuthentication(OpenIdConnectDefaults.AuthenticationScheme)
                .AddMicrosoftIdentityWebApp(options =>
                {
                    options.ClientId = _configuration.GetValue<string>("B2C_CLIENT_ID");
                    options.Domain = $"{_configuration.GetValue<string>("B2C_TENANT_NAME")}.onmicrosoft.com";
                    options.Instance = $"https://{_configuration.GetValue<string>("B2C_TENANT_NAME")}.b2clogin.com";
                    options.TenantId = _configuration.GetValue<string>("B2C_TENANT_ID");
                    options.SignUpSignInPolicyId = "b2c_1_signup_signin";
                    options.ResetPasswordPolicyId = "b2c_1_passwordreset";
                    options.ResponseType = OpenIdConnectResponseType.Code;
                    options.Scope.Add(options.ClientId);
                });

We are using PKCE for signin with B2C, using "single page application" setup. For your scenario you may need to add things like a ClientSecret as well.

@AleksandrAlbert
Copy link

AleksandrAlbert commented Jul 29, 2021

@tedvanderveen Hey Ted, I have the actual MicrosoftIdentity part working from before. I am trying to integrate into an existing application, so trying to figure out how to get Piranha to use the current Login and LogOff endpoints that I have. My Startup looks like this currently:

...

services.AddPiranha(options =>
    {
        options.AddRazorRuntimeCompilation = true;
        options.UseStartpageRouting = false;
    
        options.UseFileStorage(naming: Piranha.Local.FileStorageNaming.UniqueFolderNames);
        options.DisableRouting();
        options.UseImageSharp();
        options.UseManager();
        options.UseTinyMCE();
        options.UseMemoryCache();
        
        options.UseEF<SQLServerDb>(db =>
            db.UseSqlServer(Configuration.GetConnectionString("DbBlogConnection")));
        options.UseIdentity<IdentitySQLServerDb>(
            db => db.UseSqlServer(Configuration.GetConnectionString("DbBlogConnection")),
            null
            ,s => {
                s.LoginPath = "/Account/Login";
                s.LogoutPath = "/Account/LogOff";
                s.ForwardChallenge = OpenIdConnectDefaults.AuthenticationScheme;
                s.ReturnUrlParameter = Configuration.GetSection("AzureAd").GetValue<string>("PostLogoutRedirectUri");
            }
        );
    });

services.AddAuthentication(options =>
    {
        options.DefaultScheme = OpenIdConnectDefaults.AuthenticationScheme;
        options.DefaultSignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
    })
    .AddMicrosoftIdentityWebApp(Configuration.GetSection("AzureAd"));

...

When I try to go to manager, or to any page with an [Authorize] attribute, it sends me into a loop of redirects to the Login endpoint and never actually logs in. Could I see how you set up AddPiranha, as well as any other Claim mappings, controllers, or middleware you may have used? Thanks for the help!

@AleksandrAlbert
Copy link

AleksandrAlbert commented Jul 31, 2021

I wanted to follow up on this to say I got the azure login to work by adding some claims for the manager to the identity on authentication. This is what I did, hope it will be of help to someone else in the future:

Startup.cs

services.AddPiranha(options =>
{
    options.AddRazorRuntimeCompilation = true;
    options.UseStartpageRouting = false;

    options.UseFileStorage(naming: Piranha.Local.FileStorageNaming.UniqueFolderNames);
    options.DisableRouting();
    options.UseImageSharp();
    options.UseManager();
    options.UseTinyMCE();
    options.UseMemoryCache();

    options.UseEF<SQLServerDb>(db =>
        db.UseSqlServer(Configuration.GetConnectionString("DbConnection")));
    options.UseIdentityWithSeed<IdentitySQLServerDb>(
        db => db.UseSqlServer(Configuration.GetConnectionString("DbConnection")),
        null,
        cookie =>
        {
            cookie.LoginPath = "/Account/Login"; // Path to your login endpoint
            cookie.LogoutPath = "/Account/LogOff"; // Path to your logout endpoint
            cookie.ForwardDefault = OpenIdConnectDefaults.AuthenticationScheme; // The Scheme you want to use
        }
    );
});

services.AddAuthentication(options =>
    {
        options.DefaultScheme = OpenIdConnectDefaults.AuthenticationScheme;
        options.DefaultSignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
    })
    .AddMicrosoftIdentityWebApp(Configuration.GetSection("AzureAd"));
    

This allowed me to login to my application using AzureAD, but I was missing the claims needed to login to the manager. To add these, I implemented IClaimsTransformer. My implementation just adds the claims to any logged in user. You may want to filter or restrict this based on your requirements:

public class AddPiranhaClaims : IClaimsTransformation
    {

        public AddPiranhaClaims()
        {
        }

        public async Task<ClaimsPrincipal> TransformAsync(ClaimsPrincipal principal)
        {
            await Task.CompletedTask;

            // Clone current identity
            var clone = principal.Clone();
            
            var newIdentity = (ClaimsIdentity)clone.Identity;

            // Support AD and local accounts
            var nameId = principal.Claims.FirstOrDefault(c => c.Type == ClaimTypes.NameIdentifier ||
                                                              c.Type == ClaimTypes.Name);
            if (nameId == null)
            {
                return principal;
            }

            // Get user from database

            // Add manager claims to cloned identity
            foreach (var permission in Piranha.Manager.Permission.All())
            {
                newIdentity.AddClaim(new Claim(permission, permission));
            }
            
            // Add security claims to cloned identity
            foreach (var permission in Piranha.Security.Permission.All())
            {
                newIdentity.AddClaim(new Claim(permission, permission));
            }

            // Add identity claims to cloned identity
            foreach (var permission in Piranha.AspNetCore.Identity.Permissions.All())
            {
                newIdentity.AddClaim(new Claim(permission, permission));
            }

            return clone;
        }
    }

Then I just injected this in Startup:

services.AddScoped<IClaimsTransformation, AddPiranhaClaims>();

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.

4 participants