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

Issue With OIDC Authentication #412

Open
grumpykiwi opened this issue Aug 12, 2022 · 2 comments
Open

Issue With OIDC Authentication #412

grumpykiwi opened this issue Aug 12, 2022 · 2 comments

Comments

@grumpykiwi
Copy link

After quite a lot of fiddling to get things working. OIDC authentication is at least getting back to the home page. But now it shows a message saying it is not configured with a link to the about page where I see a big old error.

The summary error is:

The provided identity of type 'System.Security.Claims.ClaimsIdentity' is marked IsAuthenticated = true but does not have a value for Name. By default, the antiforgery system requires that all authenticated identities have a unique Name. If it is not possible to provide a unique Name for this identity, consider extending IAntiforgeryAdditionalDataProvider by overriding the DefaultAntiforgeryAdditionalDataProvider or a custom type that can provide some form of unique identifier for the current user.

I am using an app registration from Azure AD to authenticate. Is there a specific setting I need to adjust in AAD to make this work ?

Here is the relevant config

  "Security": {
    "provider": "OIDC",
    "apiKey": "",
    "viewEverythingGroups": "Domain Admins",
    "adminEverythingGroups": "Domain Admins",
    "scopes": [
      "email"
    ],
    "clientId": "<client Id>",
    "clientSecret": "<client secret>",
    "authorizationUrl": "https://login.microsoftonline.com/<tenant Id>/oauth2/v2.0/authorize",
    "accessTokenUrl": "https://login.microsoftonline.com/<tenant Id>/oauth2/v2.0/token",
    "userInfoUrl": "https://graph.microsoft.com/oidc/userinfo"
  },

Any ideas on what I might have mis-configured?

To be honest it was a bit of a chore getting this far. The documentation on this is a bit sparse.

Thanks

Mark

@itssimple
Copy link
Contributor

itssimple commented Nov 22, 2024

I've come a bit further, so what you need to do, is enable some optional claims in your OIDC application in Entra, and in my case, I added email, and set my config like this

{
  "provider": "OIDC",
  "viewEverythingGroups": "Domain Users",
  "adminEverythingGroups": "Opserver Admins",
  "scopes": [ "openid", "email" ],
  "clientId": "<clientId>",
  "clientSecret": "<clientSecret>",
  "authorizationUrl": "https://login.microsoftonline.com/<tenant>/oauth2/v2.0/authorize",
  "accessTokenUrl": "https://login.microsoftonline.com/<tenant>/oauth2/v2.0/token",
  "userInfoUrl": "https://graph.microsoft.com/oidc/userinfo",
  "nameClaim": "email",
  "groupsClaim": "groups"
}

Didn't get the groupsClaim to work yet, but I'm currently debugging the code as I'm troubleshooting it.

Edit:

Ah, groups doesn't work since they switched to the Microsoft Graph OIDC endpoint, which won't return groups no matter what you do.

So you might need to either patch the code, and add support for running against https://graph.microsoft.com/v1.0/me/transitiveMemberOf/microsoft.graph.group

@itssimple
Copy link
Contributor

Managed to get this working by modifying the auth for OIDC, to allow a separate call for groups (only valid for Microsoft Graph) for Entra OIDC.

New settings:

{
  "provider": "OIDC",
  "viewEverythingGroups": "Opserver-Read",
  "adminEverythingGroups": "Opserver-Admin",
  "scopes": [ "email", "profile", "offline_access" ],
  "clientId": "<clientId>",
  "clientSecret": "<clientSecret>",
  "authorizationUrl": "https://login.microsoftonline.com/<tenant>/oauth2/v2.0/authorize",
  "accessTokenUrl": "https://login.microsoftonline.com/<tenant>/oauth2/v2.0/token",
  "userInfoUrl": "https://graph.microsoft.com/oidc/userinfo",
  "groupInfoUrl": "https://graph.microsoft.com/v1.0/me/transitiveMemberOf/microsoft.graph.group",
  "nameClaim": "name",
  "groupsClaim": "groups"
}

And then the changes I made to get it to work (this is only tested with Entra OIDC)

diff --git a/src/Opserver.Web/Controllers/AuthController.OIDC.cs b/src/Opserver.Web/Controllers/AuthController.OIDC.cs
index e9087d48..fdb6ee8d 100644
--- a/src/Opserver.Web/Controllers/AuthController.OIDC.cs
+++ b/src/Opserver.Web/Controllers/AuthController.OIDC.cs
@@ -1,10 +1,12 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Specialized;
+using System.Linq;
 using System.Net;
 using System.Runtime.Serialization;
 using System.Security.Claims;
 using System.Text.Json;
+using System.Text.Json.Serialization;
 using System.Threading.Tasks;
 using Jil;
 using Microsoft.AspNetCore.Authentication;
@@ -16,7 +18,9 @@
 using Opserver.Helpers;
 using Opserver.Security;
 using Opserver.Views.Login;
+using StackExchange.Profiling.Internal;
 using StackExchange.Utils;
+using static System.Runtime.InteropServices.JavaScript.JSType;
 using SameSiteMode = Microsoft.AspNetCore.Http.SameSiteMode;
 
 namespace Opserver.Controllers
@@ -72,8 +76,10 @@ public async Task<IActionResult> OAuthCallback(string code, string state, string
             }
 
             // hooray! we're all set, let's go fetch our access token
-            var oidcSettings = (OIDCSecuritySettings) Current.Security.Settings;
-            var scopes = oidcSettings.Scopes ?? OIDCSecuritySettings.DefaultScopes;
+            var oidcSettings = (OIDCSecuritySettings)Current.Security.Settings;
+            var scopes = (oidcSettings.Scopes ?? OIDCSecuritySettings.DefaultScopes).ToList();
+            //scopes.Remove("openid");
+
             var redirectUri = Url.Action(
                 nameof(OAuthCallback),
                 ControllerContext.ActionDescriptor.ControllerName,
@@ -146,8 +152,39 @@ public async Task<IActionResult> OAuthCallback(string code, string state, string
                 );
             }
 
+            JsonElement? groupInfo = null;
+
+            if (!oidcSettings.GroupInfoUrl.IsNullOrWhiteSpace())
+            {
+                response = await Http.Request(oidcSettings.GroupInfoUrl)
+                    .AddHeader(HeaderNames.Authorization, "Bearer " + responsePayload.AccessToken)
+                    .ExpectString()
+                    .GetAsync();
+
+                if (!response.Success)
+                {
+                    return Error(
+                        $"failed to retrieve group info. {response.StatusCode} - {response.Data}"
+                    );
+                }
+
+
+                try
+                {
+                    groupInfo = JsonSerializer.Deserialize<JsonElement>(response.Data);
+                }
+                catch (Exception ex)
+                {
+                    ex.Log();
+                    return Error(
+                        $"could not deserialize group info. {ex.Message}"
+                    );
+                }
+
+            }
+
             // convert the user info into claims
-            static IEnumerable<Claim> ConvertToClaims(string name, JsonElement jsonElement)
+            static IEnumerable<Claim> ConvertToClaims(string name, JsonElement jsonElement, JsonElement? groupElement = null)
             {
                 if (jsonElement.ValueKind == JsonValueKind.Object)
                 {
@@ -176,9 +213,18 @@ static IEnumerable<Claim> ConvertToClaims(string name, JsonElement jsonElement)
 
                 // TODO: if we need more than just strings / arrays / objects
                 // then add support here!
+
+                if (groupElement != null)
+                {
+                    var groups = groupElement!.Value.GetProperty("value").Deserialize<List<MSGraphGroup>>();
+                    foreach (var group in groups)
+                    {
+                        yield return new Claim("groups", group.DisplayName);
+                    }
+                }
             }
 
-            var claims = ConvertToClaims(null, userInfo);
+            var claims = ConvertToClaims(null, userInfo, groupInfo);
             if (!Current.Security.TryValidateToken(new OIDCToken(claims), out var claimsPrincipal))
             {
                 return Error("could not validate ID token" + responsePayload.IdToken);
@@ -215,7 +261,7 @@ private IActionResult RedirectToProvider(string returnUrl)
                     IsEssential = true
                 });
 
-            var oidcSettings = (OIDCSecuritySettings) Current.Security.Settings;
+            var oidcSettings = (OIDCSecuritySettings)Current.Security.Settings;
             var redirectUri = Url.Action(
                 nameof(OAuthCallback),
                 ControllerContext.ActionDescriptor.ControllerName,
@@ -227,7 +273,9 @@ private IActionResult RedirectToProvider(string returnUrl)
 
             // construct the URL to the authorization endpoint
             var authorizationUrl = new UriBuilder(oidcSettings.AuthorizationUrl);
-            var scopes = oidcSettings.Scopes ?? OIDCSecuritySettings.DefaultScopes;
+            var scopes = (oidcSettings.Scopes ?? OIDCSecuritySettings.DefaultScopes).ToList();
+            //scopes.Remove("openid");
+
             var encodedState = new QueryString()
                 .Add(OidcIdentifierKey, oidcIdentifier)
                 .Add(OidcReturnUrlKey, returnUrl ?? "/");
@@ -236,6 +284,7 @@ private IActionResult RedirectToProvider(string returnUrl)
                 .Add("client_id", oidcSettings.ClientId)
                 .Add("scope", string.Join(' ', scopes))
                 .Add("redirect_uri", redirectUri)
+                .Add("response_mode", "query")
                 .Add("state", encodedState.ToUriComponent())
                 .Add("nonce", Guid.NewGuid().ToString("N"));
 
@@ -262,5 +311,18 @@ private class AccessTokenResponse
             [DataMember(Name = "token_type")]
             public string TokenType { get; set; }
         }
+
+        public class MSGraphGroup
+        {
+            [JsonPropertyName("id")]
+            public Guid Id { get; set; }
+            [JsonPropertyName("securityIdentifier")]
+            public string SecurityIdentifier { get; set; }
+            [JsonPropertyName("displayName")]
+            public string DisplayName { get; set; }
+
+            [JsonExtensionData]
+            public Dictionary<string, object> ExtraAttributes { get; set; }
+        }
     }
 }
diff --git a/src/Opserver.Web/Security/OIDCSecuritySettings.cs b/src/Opserver.Web/Security/OIDCSecuritySettings.cs
index ae4ee5ca..c7ad966c 100644
--- a/src/Opserver.Web/Security/OIDCSecuritySettings.cs
+++ b/src/Opserver.Web/Security/OIDCSecuritySettings.cs
@@ -5,7 +5,7 @@
     /// </summary>
     public class OIDCSecuritySettings : SecuritySettings
     {
-        public static readonly string[] DefaultScopes = {"openid"};
+        public static readonly string[] DefaultScopes = { "openid" };
 
         /// <summary>
         /// Gets or sets the client id for the OIDC provider.
@@ -31,6 +31,7 @@ public class OIDCSecuritySettings : SecuritySettings
         /// Gets or sets the URL used to obtain user info from the OIDC provider.
         /// </summary>
         public string UserInfoUrl { get; set; }
+        public string GroupInfoUrl { get; set; }
 
         /// <summary>
         /// Gets or sets a list of scopes to request from the OIDC provider.

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

No branches or pull requests

2 participants