opserver / Opserver

Stack Exchange's Monitoring System
https://opserver.github.io/Opserver/
MIT License
4.51k stars 827 forks source link

Claims authentication for Opserver #275

Closed AlexSikilinda closed 6 years ago

AlexSikilinda commented 7 years ago

Hi,

I'm trying to host Opserver in Azure and use Azure Active Directory (AAD) authentication. There is ActiveDirectoryProvider, but it doesn't seem to be working with AAD according to this.

So I decided to use alladmin approach with AAD handling all the authentication stuff (quite straightforward in Azure).

However, there is a minor issue in User.cs:

public User(IIdentity identity)
{
    Identity = identity;
    var i = identity as FormsIdentity; // <-- the problem
    if (i == null)
    {
        IsAnonymous = true;
        return;
    }

    IsAnonymous = !i.IsAuthenticated;
    if (i.IsAuthenticated)
        AccountName = i.Name;
}

This cast always returns null because we get ClaimsIdentity instead of FormsIdentity and thus Opserver thinks it is an anonymous request and constantly redirects to login page.

The solutions here is to remove this cast and use Identity directly:

public User(IIdentity identity)
{
    Identity = identity;
    if (Identity == null)
    {
        IsAnonymous = true;
        return;
    }

    IsAnonymous = !Identity.IsAuthenticated;
    if (Identity.IsAuthenticated)
        AccountName = Identity.Name;
}

I don't really understand this logic with identity as FormsIdentity so I'm asking if it's okay to remove this cast and if I should submit a PR here.

NickCraver commented 6 years ago

PR merged in - can you give current a try please?

AlexSikilinda commented 6 years ago

It works! Thanks.