Skip to content

Commit d18bf1c

Browse files
authored
Remove the client secret from AAD V2 authenticator (#9340)
We were only ever using the id_token which contains enough detail for NuGet.org sign in. The code response is not used. Progress on NuGet/Engineering#4099
1 parent e8c507b commit d18bf1c

3 files changed

Lines changed: 13 additions & 15 deletions

File tree

src/NuGetGallery.Services/Authentication/Providers/AzureActiveDirectoryV2/AzureActiveDirectoryV2Authenticator.cs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,13 @@ protected override void AttachToOwinApp(IGalleryConfigurationService config, IAp
101101
RedirectUri = siteRoot + _callbackPath,
102102
PostLogoutRedirectUri = siteRoot,
103103
Scope = OpenIdConnectScope.OpenIdProfile + " email",
104-
ResponseType = OpenIdConnectResponseType.CodeIdToken,
104+
ResponseType = OpenIdConnectResponseType.IdToken,
105105
TokenValidationParameters = new Microsoft.IdentityModel.Tokens.TokenValidationParameters() { ValidateIssuer = false },
106106
Notifications = new OpenIdConnectAuthenticationNotifications
107107
{
108108
AuthenticationFailed = AuthenticationFailed,
109-
RedirectToIdentityProvider = RedirectToIdentityProvider
109+
RedirectToIdentityProvider = RedirectToIdentityProvider,
110+
AuthorizationCodeReceived = AuthorizationCodeReceived,
110111
}
111112
};
112113

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

263264
// We always want to show the options to select account when signing in and while changing account.
@@ -271,5 +272,13 @@ private AuthenticationProperties GetAuthenticationPropertiesFromProtocolMessage(
271272
var authenticationPropertiesEncodedString = message.State.Split('=');
272273
return options.StateDataFormat.Unprotect(authenticationPropertiesEncodedString[1]);
273274
}
275+
276+
private Task AuthorizationCodeReceived(AuthorizationCodeReceivedNotification context)
277+
{
278+
// Explicitly set the access_token to null. The access_token is used for authorized requests to AAD on
279+
// behalf of the end user. We do not use this feature. We only use the id_token.
280+
context.HandleCodeRedemption(accessToken: null, idToken: context.JwtSecurityToken.RawData);
281+
return Task.CompletedTask;
282+
}
274283
}
275284
}

src/NuGetGallery.Services/Authentication/Providers/AzureActiveDirectoryV2/AzureActiveDirectoryV2AuthenticatorConfiguration.cs

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ namespace NuGetGallery.Authentication.Providers.AzureActiveDirectoryV2
1212
public class AzureActiveDirectoryV2AuthenticatorConfiguration : AuthenticatorConfiguration
1313
{
1414
public string ClientId { get; set; }
15-
public string ClientSecret { get; set; }
1615

1716
public AzureActiveDirectoryV2AuthenticatorConfiguration()
1817
{
@@ -31,7 +30,7 @@ public override void ApplyToOwinSecurityOptions(AuthenticationOptions options)
3130
// the auth flow.
3231
openIdOptions.AuthenticationMode = AuthenticationMode.Passive;
3332

34-
// Make sure ClientId and ClientSecret is configured
33+
// Make sure ClientId is configured
3534
if (String.IsNullOrEmpty(ClientId))
3635
{
3736
throw new ConfigurationErrorsException(String.Format(
@@ -40,16 +39,7 @@ public override void ApplyToOwinSecurityOptions(AuthenticationOptions options)
4039
"Auth.CommonAuth.ClientId"));
4140
}
4241

43-
if (String.IsNullOrEmpty(ClientSecret))
44-
{
45-
throw new ConfigurationErrorsException(String.Format(
46-
CultureInfo.CurrentCulture,
47-
ServicesStrings.MissingRequiredConfigurationValue,
48-
"Auth.CommonAuth.ClientSecret"));
49-
}
50-
5142
openIdOptions.ClientId = ClientId;
52-
openIdOptions.ClientSecret = ClientSecret;
5343
openIdOptions.Authority = String.Format(CultureInfo.InvariantCulture, AzureActiveDirectoryV2Authenticator.Authority, AzureActiveDirectoryV2Authenticator.V2CommonTenant);
5444
}
5545
}

src/NuGetGallery/Web.config

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@
101101
<add key="Auth.MicrosoftAccount.ClientSecret" value=""/>
102102
<add key="Auth.AzureActiveDirectoryV2.Enabled" value="false"/>
103103
<add key="Auth.AzureActiveDirectoryV2.ClientId" value=""/>
104-
<add key="Auth.AzureActiveDirectoryV2.ClientSecret" value=""/>
105104
<add key="Auth.AzureActiveDirectory.Enabled" value="false"/>
106105
<add key="Auth.AzureActiveDirectory.ClientId" value=""/>
107106
<add key="Auth.AzureActiveDirectory.Authority" value=""/>

0 commit comments

Comments
 (0)