3
\$\begingroup\$

I am implementing SSO in my application via Microsoft Entra. It has a client application with Cloud APIs.

The Client passes an Entra JWT to the server obtained from the MS Entra libraries. My database has a list of Users with each user under a tenant. Each tenant has only one possible Entra tenant ID, and this list is collected to tell the validator what tenants to accept in the below code (and later used with the email address to validate that the user is allowed to log in from that Entra tenant).

So for the purposes of my app's security model, is there an obvious way to pass a JWT to the below code that impersonates both the issuer and email address and could therefore send a correct tenant ID and email to the authentication code?

using Dapper;
using HealthLibrary;
using Microsoft.IdentityModel.Protocols.OpenIdConnect;
using Microsoft.IdentityModel.Protocols;
using Microsoft.IdentityModel.Tokens;
using System;
using System.Collections.Generic;
using System.Configuration;
using System.Data;
using System.Data.SqlClient;
using System.IdentityModel.Tokens.Jwt;
using System.Linq;
using System.Security.Claims;
using System.Threading;
using System.Web.Configuration;
using System.Threading.Tasks;
using System.Text.RegularExpressions;


        //called from the web service with token, IP address of user and terminal name of the user
        public async Task<ApiResponse> ValidateJwt(string token, string ip, string terminalName)
        {
            if (string.IsNullOrEmpty(token))
            {
                throw new Exception("No token was supplied");
            }

            // Get list of valid Entra tenant IDs
            List<string> validTenantIds = validTenantIDs();
            var claimsPrincipal = await ValidateToken(token, validTenantIds);

            // Extract Tenant ID
            var tenantId = claimsPrincipal.Claims.FirstOrDefault(c => c.Type == "iss")?.Value;
            if (string.IsNullOrEmpty(tenantId))
            {
                throw new Exception("No Entra tenant ID supplied");
            }
            tenantId = extractGuid(tenantId);

            // Extract Email
            var email = claimsPrincipal.Claims.FirstOrDefault(c => c.Type == "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/upn")?.Value;
            if (string.IsNullOrEmpty(email))
            {
                throw new Exception("Email missing");
            }

            return GenerateAppKey(email, tenantId, ip, terminalName);
        }

        //Gets valid tenant IDs from the database and links them with the Entra issuer URL
        private List<string> validTenantIDs()
        {
            using (SqlConnection connection = new SqlConnection(ConfigurationManager.ConnectionStrings["ssoConnection"].ConnectionString))
            {
                var procedure = WebConfigurationManager.AppSettings["DatabaseName"] + ".dbo.SSO_GetValidTenants";
                var p = new DynamicParameters();
                var results = connection.Query(procedure, p, commandType: CommandType.StoredProcedure, commandTimeout: 120).ToList();
                List<string> list = new List<string>();
                foreach (var result in results)
                {
                    list.Add("https://sts.windows.net/" + result.MicrosoftEntraTenantID + "/");
                }
                return list;
            }
        }

        //Extract the GUID from the Entra URL for validation purposes later
        private static string extractGuid(string url)
        {
            //extract the GUID from the Entra URL 
            var pattern = @"https:\/\/sts\.windows\.net\/([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})\/";
            var match = Regex.Match(url, pattern);

            if (match.Success && match.Groups.Count > 1)
            {
                return match.Groups[1].Value;
            }

            throw new Exception("No GUID could be identified from the Azure URL. Please contact your administrator for assistance");
        }

        //validates the token and returns a ClaimsPrincipal object
        private async Task< ClaimsPrincipal> ValidateToken(string token, List<string> validTenantIds)
        {
            string invalidTokenMessage = "A sign in was attempted for an invalid Entra tenant."
                                        + Environment.NewLine
                                        + "Your site may not be set up for Single Sign On - please contact your Administrator for more details.";
            try
            {
                var tokenHandler = new JwtSecurityTokenHandler();
                var configurationManager = new ConfigurationManager<OpenIdConnectConfiguration>(
                "https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration",
                new OpenIdConnectConfigurationRetriever());

                var openIdConfig = configurationManager.GetConfigurationAsync(CancellationToken.None);
                var validationParameters = new TokenValidationParameters
                {
                    IssuerSigningKeys = openIdConfig.Result.SigningKeys,
                    ValidIssuers = validTenantIds,
                    ValidateIssuer = true,
                    ValidateAudience = true,
                    ValidateLifetime = true,
                    ValidAudience = "api://" + WebConfigurationManager.AppSettings["EntraClientID"]
                };

                return tokenHandler.ValidateToken(token, validationParameters, out _);
            }
            catch(SecurityTokenInvalidIssuerException)
            {
                throw new Exception(invalidTokenMessage);
            }
        }
\$\endgroup\$
3
  • \$\begingroup\$ There's so much missing here that I hardly understand how to review this snippet. Please add your using statements at least. As a guess, ValidateToken only catches SecurityTokenInvalidIssuerException to rethrow it as generic Exception, and all other validation exceptions (I assume there's also SecurityTokenInvalidAudienceException, for example, unless the library is doing something in a deeply wrong/confusing way) are kept as-is - is that intentional? \$\endgroup\$ Commented Apr 14 at 23:53
  • \$\begingroup\$ Hey @STerliakov thanks for the feedback, I've added the using statements. I catch that exception to throw it back to the user with a different message, because that one is a specific scenario that can happen in Production if their site isn't signed up for SSO yet - all others just use the standard message. The main relevant library to my validation is Microsoft.IdentityModel \$\endgroup\$ Commented Apr 15 at 1:42
  • \$\begingroup\$ I should add that this code is not Production ready, I am trying to find issues with the basic validation of the token before I build around it. In theory either the current validation using the library will validate a spoofed token or it won't - I'm trying to determine whether this approach is appropriate before I go further \$\endgroup\$ Commented Apr 15 at 1:44

1 Answer 1

4
\$\begingroup\$

Here are some notes :

  1. token must be parsed into JwtSecurityToken object. If it's cannot be parsed, then there is an issue with the token.

  2. validTenantIDs can be cached since you need to initiate once, and reuse throughout your application lifecycle. You do not need to call your database each time you validate. If your Tenants are often updated, then try to see a way to update the cache once the there is an update on your tenants. (you can use distributed cache (e.g. Redis) or memory cache).

  3. ValidateToken you only catch and ignore SecurityTokenInvalidIssuerException and ignore all other exceptions. What you need is to catch all exceptions, log them, and monitor them. It's important to do that for debugging purposes and also to avoid any potential security risk.

  4. TokenValidationParameters can be cached once, and reused throughout your application lifecycle.

  5. you obtained the tenantId from iss claim, but you can take it from tid claim which would be the Tenant ID Guid, in which extractGuid will be no longer needed.

  6. When the code throw an exception it means one of the following :

    1. The code found a new case that needs to be handled.
    2. The code could not handle that error since it's out of control (e.g. server errors) and requires someone attention.
    3. The code was implemented by a careless developer who loves to leave some puzzles for others to be solved (in other word, clean her/his mess).

Suggestions :

  1. Implement a singleton JWT handler class that can validate tokens and perhaps if possible generate new ones too.

  2. Use result type object in your controller to return either success or failure responses to the consumer.

  3. Use try/catch to catch unexpected errors, and log them all to make it easier to debug, and return a failure state to the consumer with a generic error message to conceal your error details from the user.

  4. Controllers should only handle requests, so, do not mix your application logic with the requests logic.

  5. Use jwt.io to decode a token, and find out which property (claim) that you want to grasp.

\$\endgroup\$
4
  • \$\begingroup\$ Thank you, I'll implement these and log my errors. To confirm, the logic is "valid" in the sense that it will check the token is properly signed from Microsoft and not some random third party? \$\endgroup\$ Commented Apr 16 at 0:03
  • \$\begingroup\$ @Thomas the logic will only validate JWT token against whatever configuration you passed with TokenValidationParameters. if they matched, then it's a valid JWT token. However, that does not guarantee the authenticity of the user. you need to also set some security measurements such as cookies or sessions for the user once logged in via your application alongside the JWT token to verify that user is logged in via your portal and not from outside it. \$\endgroup\$ Commented Apr 16 at 3:42
  • \$\begingroup\$ yes the end result of this validation code is (if successful) a token for my application, I am not that new to dev, just JWT authentication :) So for my understanding, is it the IssuerSigningKeys = openIdConfig.Result.SigningKeys, line that causes it to validate the JWT is correctly signed by a valid Microsoft Entra private key? \$\endgroup\$ Commented Apr 18 at 9:46
  • \$\begingroup\$ @Thomas yes it's part of the validation along with the token expiration and other claims as well. \$\endgroup\$ Commented Apr 18 at 10:18

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.