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.

Sign up for GitHub

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

Jump to bottom

Remove the client secret from AAD V2 authenticator #9340

Merged
merged 3 commits into from
Jan 9, 2023
Merged

Remove the client secret from AAD V2 authenticator #9340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
74760e3
Remove the client secret from AAD V2 authenticator
joelverhagen Jan 4, 2023
9274b4f
Remove secret config
joelverhagen Jan 5, 2023
d648bf9
Fix comment
joelverhagen Jan 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions ...es/Authentication/Providers/AzureActiveDirectoryV2/AzureActiveDirectoryV2Authenticator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,13 @@ protected override void AttachToOwinApp(IGalleryConfigurationService config, IAp
RedirectUri = siteRoot + _callbackPath,
PostLogoutRedirectUri = siteRoot,
Scope = OpenIdConnectScope.OpenIdProfile + " email",
ResponseType = OpenIdConnectResponseType.CodeIdToken,
ResponseType = OpenIdConnectResponseType.IdToken,
TokenValidationParameters = new Microsoft.IdentityModel.Tokens.TokenValidationParameters() { ValidateIssuer = false },
Notifications = new OpenIdConnectAuthenticationNotifications
{
AuthenticationFailed = AuthenticationFailed,
RedirectToIdentityProvider = RedirectToIdentityProvider
RedirectToIdentityProvider = RedirectToIdentityProvider,
AuthorizationCodeReceived = AuthorizationCodeReceived,
}
};

Expand Down Expand Up @@ -257,7 +258,7 @@ private Task RedirectToIdentityProvider(RedirectToIdentityProviderNotification<O
// Set the redirect_uri token for the alternate domains of same gallery instance
if (_alternateSiteRootList != null && _alternateSiteRootList.Contains(notification.Request.Uri.Host))
{
notification.ProtocolMessage.RedirectUri = "https://" + notification.Request.Uri.Host + "/" + _callbackPath ;
notification.ProtocolMessage.RedirectUri = "https://" + notification.Request.Uri.Host + "/" + _callbackPath;
}

// We always want to show the options to select account when signing in and while changing account.
Expand All @@ -271,5 +272,13 @@ private AuthenticationProperties GetAuthenticationPropertiesFromProtocolMessage(
var authenticationPropertiesEncodedString = message.State.Split('=');
return options.StateDataFormat.Unprotect(authenticationPropertiesEncodedString[1]);
}

private Task AuthorizationCodeReceived(AuthorizationCodeReceivedNotification context)
{
// Explicitly set the access_token to null. The access_token is used for authorized requests to AAD on
// behalf of the end user. We do not use this feature. We only use the id_token.
context.HandleCodeRedemption(accessToken: null, idToken: context.JwtSecurityToken.RawData);
return Task.CompletedTask;
}
}
}
12 changes: 1 addition & 11 deletions ...tion/Providers/AzureActiveDirectoryV2/AzureActiveDirectoryV2AuthenticatorConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ namespace NuGetGallery.Authentication.Providers.AzureActiveDirectoryV2
public class AzureActiveDirectoryV2AuthenticatorConfiguration : AuthenticatorConfiguration
{
public string ClientId { get; set; }
public string ClientSecret { get; set; }

public AzureActiveDirectoryV2AuthenticatorConfiguration()
{
Expand All @@ -31,7 +30,7 @@ public override void ApplyToOwinSecurityOptions(AuthenticationOptions options)
// the auth flow.
openIdOptions.AuthenticationMode = AuthenticationMode.Passive;

// Make sure ClientId and ClientSecret is configured
// Make sure ClientId is configured
if (String.IsNullOrEmpty(ClientId))
{
throw new ConfigurationErrorsException(String.Format(
Expand All @@ -40,16 +39,7 @@ public override void ApplyToOwinSecurityOptions(AuthenticationOptions options)
"Auth.CommonAuth.ClientId"));
}

if (String.IsNullOrEmpty(ClientSecret))
{
throw new ConfigurationErrorsException(String.Format(
CultureInfo.CurrentCulture,
ServicesStrings.MissingRequiredConfigurationValue,
"Auth.CommonAuth.ClientSecret"));
}

openIdOptions.ClientId = ClientId;
openIdOptions.ClientSecret = ClientSecret;
openIdOptions.Authority = String.Format(CultureInfo.InvariantCulture, AzureActiveDirectoryV2Authenticator.Authority, AzureActiveDirectoryV2Authenticator.V2CommonTenant);
}
}
Expand Down
1 change: 0 additions & 1 deletion src/NuGetGallery/Web.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@
<add key="Auth.MicrosoftAccount.ClientSecret" value=""/>
<add key="Auth.AzureActiveDirectoryV2.Enabled" value="false"/>
<add key="Auth.AzureActiveDirectoryV2.ClientId" value=""/>
<add key="Auth.AzureActiveDirectoryV2.ClientSecret" value=""/>
<add key="Auth.AzureActiveDirectory.Enabled" value="false"/>
<add key="Auth.AzureActiveDirectory.ClientId" value=""/>
<add key="Auth.AzureActiveDirectory.Authority" value=""/>
Expand Down