Skip to content

Commit f94dace

Browse files
committed
Service principal authentication with client secret
Adds the option to specify a client secret in the endpoint credential structures read from the ARTIFACTS_CREDENTIALPROVIDER_FEED_ENDPOINTS environment variable. A client certificate may be difficult to use in some CICD scenarios, so this adds another option for service principal authentication. Users should still prefer certificates over secrets. Signed-off-by: Niko Lappalainen <niko.lappalainen@m-files.com>
1 parent e479cc5 commit f94dace

5 files changed

Lines changed: 59 additions & 19 deletions

File tree

CredentialProvider.Microsoft/CredentialProviders/VstsBuildTaskServiceEndpoint/VstsBuildTaskServiceEndpointCredentialProvider.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ public override async Task<GetAuthenticationCredentialsResponse> HandleRequestAs
101101
InteractiveTimeout = TimeSpan.FromSeconds(EnvUtil.GetDeviceFlowTimeoutFromEnvironmentInSeconds(Logger)),
102102
ClientId = matchingEndpoint.ClientId,
103103
ClientCertificate = clientCertificate,
104-
TenantId = authInfo.EntraTenantId
104+
TenantId = authInfo.EntraTenantId,
105+
ClientSecret = !string.IsNullOrEmpty(matchingEndpoint.ClientSecret) ? new(matchingEndpoint.ClientSecret) : null,
105106
};
106107

107108
foreach(var tokenProvider in tokenProviders)

CredentialProvider.Microsoft/Util/FeedEndpointCredentialsParser.cs

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Linq;
23
using System.Collections.Generic;
34
using System.Text.Json;
45
using System.Text.Json.Serialization;
@@ -32,6 +33,8 @@ public class EndpointCredentials
3233
public string CertificateFilePath { get; set; }
3334
[JsonPropertyName("clientCertificateSubjectName")]
3435
public string CertificateSubjectName { get; set; }
36+
[JsonPropertyName("clientSecret")]
37+
public string ClientSecret { get; set; }
3538
}
3639

3740
public class EndpointCredentialsContainer
@@ -67,45 +70,52 @@ public static Dictionary<string, EndpointCredentials> ParseFeedEndpointsJsonToDi
6770
return credsResult;
6871
}
6972

70-
foreach (var credentials in endpointCredentials.EndpointCredentials)
73+
// Filter out and log invalid credentials, but keep valid ones even if some are invalid
74+
var validCredentials = endpointCredentials.EndpointCredentials.Where(credentials =>
7175
{
7276
if (credentials == null)
7377
{
7478
logger.Verbose(Resources.EndpointParseFailure);
75-
break;
79+
return false;
7680
}
7781

7882
if (credentials.ClientId == null)
7983
{
8084
logger.Verbose(Resources.EndpointParseFailure);
81-
break;
85+
return false;
8286
}
8387

8488
if (credentials.CertificateSubjectName != null && credentials.CertificateFilePath != null)
8589
{
8690
logger.Verbose(Resources.EndpointParseFailure);
87-
break;
91+
return false;
8892
}
89-
90-
if (!Uri.TryCreate(credentials.Endpoint, UriKind.Absolute, out var endpointUri))
93+
return true;
94+
}).Select(credentials => new
95+
{
96+
ParsedEndpoint = Uri.TryCreate(credentials.Endpoint, UriKind.Absolute, out var uri) ? uri.AbsoluteUri : null,
97+
Credentials = credentials
98+
}).Where(c =>
99+
{
100+
if (c.ParsedEndpoint is null)
91101
{
92102
logger.Verbose(Resources.EndpointParseFailure);
93-
break;
94-
}
95-
96-
var urlEncodedEndpoint = endpointUri.AbsoluteUri;
97-
if (!credsResult.ContainsKey(urlEncodedEndpoint))
98-
{
99-
credsResult.Add(urlEncodedEndpoint, credentials);
103+
return false;
100104
}
105+
return true;
106+
});
107+
foreach (var item in validCredentials)
108+
{
109+
if (!credsResult.ContainsKey(item.ParsedEndpoint))
110+
credsResult.Add(item.ParsedEndpoint, item.Credentials);
101111
}
102112

103113
return credsResult;
104114
}
105115
catch (Exception ex)
106116
{
107117
logger.Verbose(string.Format(Resources.VstsBuildTaskExternalCredentialCredentialProviderError, ex));
108-
return new Dictionary<string, EndpointCredentials>(StringComparer.OrdinalIgnoreCase); ;
118+
return new Dictionary<string, EndpointCredentials>(StringComparer.OrdinalIgnoreCase);
109119
}
110120
}
111121

README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,13 +157,14 @@ The Credential Provider accepts a set of environment variables. Not all of them
157157

158158
- `ARTIFACTS_CREDENTIALPROVIDER_FEED_ENDPOINTS`: Json that contains an array of endpoints, usernames and azure service principal information needed to authenticate to Azure Artifacts feed endponts. Example:
159159
```javascript
160-
{"endpointCredentials": [{"endpoint":"http://example.index.json", "clientId":"required", "clientCertificateSubjectName":"optional", "clientCertificateFilePath":"optional"}]}
160+
{"endpointCredentials": [{"endpoint":"http://example.index.json", "clientId":"required", "clientCertificateSubjectName":"optional", "clientCertificateFilePath":"optional", "clientSecret":"optional"}]}
161161
```
162162

163163
- `endpoint`: Required. Feed url to authenticate.
164164
- `clientId`: Required for both Azure Managed Identites and Service Principals. For user assigned managed identities enter the Entra client id. For system assigned managed identities set the value to `system`.
165165
- `clientCertificateSubjectName`: Subject Name of the certificate located in the CurrentUser or LocalMachine certificate store. Optional field. Only used for service principal authentication.
166-
- `clientCertificateFilePath`: File path location of the certificate on the machine. Optional field. Only used by service principal authentication.
166+
- `clientCertificateFilePath`: File path location of the certificate on the machine. Optional field. Only used by service principal authentication.
167+
- `clientSecret`: Client secret used for service principal authentication. Optional field. Only used for sevice principal authentication if client certificate is not specified. Prefer using a certificate for improved security.
167168

168169
## Release version 1.0.0
169170

src/Authentication/MsalServicePrincipalTokenProvider.cs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public MsalServicePrincipalTokenProvider(IPublicClientApplication app, ILogger l
2121
public bool CanGetToken(TokenRequest tokenRequest)
2222
{
2323
return !string.IsNullOrWhiteSpace(tokenRequest.ClientId)
24-
&& tokenRequest.ClientCertificate != null;
24+
&& (tokenRequest.ClientCertificate != null || tokenRequest.ClientSecret != null);
2525
}
2626

2727
public async Task<AuthenticationResult?> GetTokenAsync(TokenRequest tokenRequest, CancellationToken cancellationToken = default)
@@ -37,7 +37,7 @@ public bool CanGetToken(TokenRequest tokenRequest)
3737
var app = ConfidentialClientApplicationBuilder.Create(tokenRequest.ClientId)
3838
.WithHttpClientFactory(appConfig.HttpClientFactory)
3939
.WithLogging(appConfig.LoggingCallback, appConfig.LogLevel, appConfig.EnablePiiLogging, appConfig.IsDefaultPlatformLoggingEnabled)
40-
.WithCertificate(tokenRequest.ClientCertificate, sendX5C: true)
40+
.WithCertificateOrClientSecret(tokenRequest)
4141
.WithTenantId(tokenRequest.TenantId)
4242
.Build();
4343

@@ -54,4 +54,22 @@ public bool CanGetToken(TokenRequest tokenRequest)
5454
}
5555
}
5656
}
57+
58+
public static class MsalApplicationBuilderExtensions
59+
{
60+
public static ConfidentialClientApplicationBuilder WithCertificateOrClientSecret(
61+
this ConfidentialClientApplicationBuilder builder,
62+
TokenRequest tokenRequest
63+
)
64+
{
65+
if (tokenRequest.ClientCertificate != null)
66+
{
67+
return builder.WithCertificate(tokenRequest.ClientCertificate, sendX5C: true);
68+
}
69+
else
70+
{
71+
return builder.WithClientSecret(tokenRequest.ClientSecret?.GetSecretString()!);
72+
}
73+
}
74+
}
5775
}

src/Authentication/TokenRequest.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,14 @@ public TokenRequest()
4040
public string? TenantId { get; set; } = null;
4141

4242
public X509Certificate2? ClientCertificate { get; set; } = null;
43+
44+
public ClientSecret? ClientSecret { get; set; } = null;
45+
}
46+
47+
/// <summary>
48+
/// Wraps a client secret string to avoid accidental logging.
49+
/// </summary>
50+
public class ClientSecret(string secret)
51+
{
52+
public string GetSecretString() => secret;
4353
}

0 commit comments

Comments
 (0)