You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Jim Spring <no...@github.com> on 2016/03/29 21:50:56 UTC
[jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Per discussion with @nacx regarding jclouds-labs PR https://github.com/jclouds/jclouds-labs/pull/250, isolating Azure AD Oauth changes into the JClouds oauth module.
Per README, tested as follows:
```
mvn clean install -Plive \
-Dtest.oauth.identity=<email addr> \
-Dtest.oauth.credential=<private key> \
-Dtest.oauth.endpoint=https://accounts.google.com/o/oauth2/token \
-Dtest.jclouds.oauth.audience=https://accounts.google.com/o/oauth2/token \
-Dtest.jclouds.oauth.scope=https://www.googleapis.com/auth/prediction \
-Dtest.oauth.azure-identity=<azure app id> \
-Dtest.oauth.azure-credential=<azure app password> \
-Dtest.oauth.azure-endpoint=https://login.microsoftonline.com/<tenant id>/oauth2/token
```
Steps for creating the Service Principal and Application, the Credential, and finding the Tenant Id are [here](https://azure.microsoft.com/en-us/documentation/articles/resource-group-authenticate-service-principal/).
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/941
-- Commit Summary --
* Add support for Azure AD authentication using Service Principal and Password
-- File Changes --
M apis/oauth/README (15)
M apis/oauth/src/main/java/org/jclouds/oauth/v2/AuthorizationApi.java (11)
M apis/oauth/src/main/java/org/jclouds/oauth/v2/config/CredentialType.java (5)
M apis/oauth/src/main/java/org/jclouds/oauth/v2/config/OAuthModule.java (6)
M apis/oauth/src/main/java/org/jclouds/oauth/v2/config/OAuthProperties.java (29)
A apis/oauth/src/main/java/org/jclouds/oauth/v2/domain/ClientSecret.java (49)
A apis/oauth/src/main/java/org/jclouds/oauth/v2/filters/ClientCredentialsSecretFlow.java (100)
A apis/oauth/src/test/java/org/jclouds/oauth/v2/AuthorizationApiAzureLiveTest.java (88)
M apis/oauth/src/test/java/org/jclouds/oauth/v2/AuthorizationApiLiveTest.java (5)
M apis/oauth/src/test/java/org/jclouds/oauth/v2/AuthorizationApiMockTest.java (45)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/941.patch
https://github.com/jclouds/jclouds/pull/941.diff
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
> + assertNotNull(token, "no token when authorizing password based credential");
> + }
> +
> + /** OAuth isn't registered as a provider intentionally, so we fake one. */
> + @Override protected ProviderMetadata createProviderMetadata() {
> + return forApiOnEndpoint(AuthorizationApi.class, endpoint).toBuilder().id("oauth").build();
> + }
> +
> + @Override protected Properties setupProperties() {
> + Properties props = super.setupProperties();
> + props.put(CREDENTIAL_TYPE, CredentialType.CLIENT_CREDENTIALS_SECRET.toString());
> + resource = "https://management.azure.com/";
> + props.put(RESOURCE, resource);
> + endpoint = checkNotNull(setIfTestSystemPropertyPresent(props, OAuthProperties.AZURE_ENDPOINT), OAuthProperties.AZURE_ENDPOINT);
> + identity = checkNotNull(setIfTestSystemPropertyPresent(props, OAuthProperties.AZURE_IDENTITY), OAuthProperties.AZURE_IDENTITY);
> + credential = checkNotNull(setIfTestSystemPropertyPresent(props, OAuthProperties.AZURE_CREDENTIAL), OAuthProperties.AZURE_CREDENTIAL);
Change these properties to use the jclouds default ones.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/f375a9fa56cf71f32d857e40f5b93b3e2ef9e9b2#r57816187
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -36,6 +36,22 @@
> */
> public static final String CREDENTIAL_TYPE = "jclouds.oauth.credential-type";
>
> + /**
> + * The client_credentials grant type allows for an optional "scope" to be included.
> + *
> + * @see <a href="https://tools.ietf.org/html/rfc6749#section-4.4.2">doc</a>
> + */
> + public static final String SCOPE = "jclouds.oauth.scope";
Looks like this constant is used only in tests? Scopes in this API are configured per provider by binding an `OAuthScopes` instance. For example, in the [gogole-compute-engine provider](https://github.com/jclouds/jclouds/blob/master/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/config/GoogleComputeEngineHttpApiModule.java#L70-L73) we bind the scopes based on the permissions we want for the access token, according to [their api docs ](https://cloud.google.com/compute/docs/api/how-tos/authorization#user_auth_flow).
In the case of Azure, it looks like the parameter is not used (or that the common practice is to not send it) and the access token is granted a default scope. I've read somewhere that default scope in Azure, which is informed in the authentication response, is `user_impersonation`.
For consistency, I would suggest the following:
* Remove this property, as this should be configured by binding a `OAuthScopes` instance.
* Then either:
* Mark the `OAuthScopes` as optional for injection, so a binding is not required. In this case, the Azure provider won't bind any scope and the authorization request won't send the "scopes" parameter.
* Keep the `OAuthScopes` mandatory and, in Azure, configure the [AzureComputeHttpApiModule](https://github.com/ritazh/jclouds-labs/blob/azurecompute-arm/azurecompute-arm/src/main/java/org/jclouds/azurecomputearm/config/AzureComputeHttpApiModule.java#L56) to bind the scopes as follows, in order to request a token with the Azure default scope grant (note that this approach will cause the authentication requests to be sent with the parameter `scope=user_impersonation`. I'm not very familiar with how OAuth works in Azure and I'm not sure this would be the right way to request the token):
```java
bind(OAuthScopes.class).toInstance(OAuthScopes.SingleScope.create("user_impersonation"));
```
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/4bb749e1a41d5818a8c5e327b92096dfe984e0b4#r58027837
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Jim Spring <no...@github.com>.
> + */
> + public static final String AZURE_CREDENTIAL = "oauth.azure-credential";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, an "azure-identity"
> + * is the GUID associated with an application given permssion to perform operations in
> + * Azure Resource Manager. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_IDENTITY = "oauth.azure-identity";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, the azure-endpoint
> + * is the URL https://login.microsoftonline.com/<Tenant ID>/oauth2/token, where <Tenant ID>
> + * corresponds to your Azure AD Tenant. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_ENDPOINT = "oauth.azure-endpoint";
The question was specifically about live test. Google credentials will fail on azure and vice versa.
Sent from my iThingy
> On Mar 29, 2016, at 16:05, Ignasi Barrera <no...@github.com> wrote:
>
> In apis/oauth/src/main/java/org/jclouds/oauth/v2/config/OAuthProperties.java:
>
> > + */
> > + public static final String AZURE_CREDENTIAL = "oauth.azure-credential";
> > +
> > + /**
> > + * When using oauth with Azure Active Direction and Client Credentials, an "azure-identity"
> > + * is the GUID associated with an application given permssion to perform operations in
> > + * Azure Resource Manager. Only used for OAuth Live Tests.
> > + */
> > + public static final String AZURE_IDENTITY = "oauth.azure-identity";
> > +
> > + /**
> > + * When using oauth with Azure Active Direction and Client Credentials, the azure-endpoint
> > + * is the URL https://login.microsoftonline.com/<Tenant ID>/oauth2/token, where <Tenant ID>
> > + * corresponds to your Azure AD Tenant. Only used for OAuth Live Tests.
> > + */
> > + public static final String AZURE_ENDPOINT = "oauth.azure-endpoint";
> Following the discussion in the Azure PR, I don't think we need this Azure specific properties. We should be able to use the already existing identity and credential, and the oauth.endpoint property for this purpose.
>
> Looking at the code, the value of the azure specific endpoint property is not injected in the authentication flow, so it is not used. If I'm not wrong, live tests just bind its value to the oauth.endpoint property to make it work, but that only tells the property is redundant. Same happens to the identity and credential properties. The authentication flow uses the Supplier<Credentials> (which provide the identity and credential configured when creating the context), so these properties don't take effect.
>
> I'd say we could get rid of them and say that using Azure ARM just requires users to create the context using the App id and password as the credentials, and setting the oauth.endpoint property taking into account the tenant id?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly or view it on GitHub
>
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/f375a9fa56cf71f32d857e40f5b93b3e2ef9e9b2#r57816965
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> + public static final String AZURE_CREDENTIAL = "oauth.azure-credential";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, an "azure-identity"
> + * is the GUID associated with an application given permssion to perform operations in
> + * Azure Resource Manager. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_IDENTITY = "oauth.azure-identity";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, the azure-endpoint
> + * is the URL https://login.microsoftonline.com/<Tenant ID>/oauth2/token, where <Tenant ID>
> + * corresponds to your Azure AD Tenant. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_ENDPOINT = "oauth.azure-endpoint";
Following the discussion in the Azure PR, I don't think we need this Azure specific properties. We should be able to use the already existing identity and credential, and the `oauth.endpoint` property for this purpose.
Looking at the code, the value of the azure specific endpoint property is not injected in the authentication flow, so it is not used. If I'm not wrong, live tests just bind its value to the `oauth.endpoint` property to make it work, but that only tells the property is redundant. Same happens to the identity and credential properties. The authentication flow uses the `Supplier<Credentials>` (which provide the identity and credential configured when creating the context), so these properties don't take effect.
I'd say we could get rid of them and say that using Azure ARM just requires users to create the context using the App id and password as the credentials, and setting the `oauth.endpoint` property taking into account the tenant id?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/f375a9fa56cf71f32d857e40f5b93b3e2ef9e9b2#r57815669
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Jim Spring <no...@github.com>.
> + */
> + public static final String AZURE_CREDENTIAL = "oauth.azure-credential";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, an "azure-identity"
> + * is the GUID associated with an application given permssion to perform operations in
> + * Azure Resource Manager. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_IDENTITY = "oauth.azure-identity";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, the azure-endpoint
> + * is the URL https://login.microsoftonline.com/<Tenant ID>/oauth2/token, where <Tenant ID>
> + * corresponds to your Azure AD Tenant. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_ENDPOINT = "oauth.azure-endpoint";
LiveTest could just "authenticateTest" and switch based off credential type and asset Token present after switch. Seems cleanest to me.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/f375a9fa56cf71f32d857e40f5b93b3e2ef9e9b2#r57821611
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
> + scope = resource;
> + return props;
> + }
> +
> + @Override protected Iterable<Module> setupModules() {
> + return ImmutableList.<Module>builder() //
> + .add(new OAuthModule()) //
> + .add(new Module() {
> + @Override public void configure(Binder binder) {
> + // ContextBuilder erases oauth.endpoint, as that's the same name as the provider key.
> + binder.bindConstant().annotatedWith(Names.named("oauth.endpoint")).to(endpoint);
> + binder.bind(OAuthScopes.class).toInstance(SingleScope.create(scope));
> + }
> + }).addAll(super.setupModules()).build();
> + }
> +}
If possible, get rid of this class and just add the new method to the existing live test.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/f375a9fa56cf71f32d857e40f5b93b3e2ef9e9b2#r57816361
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +import javax.inject.Inject;
> +import javax.inject.Named;
> +
> +import static java.util.concurrent.TimeUnit.SECONDS;
> +import static org.jclouds.Constants.PROPERTY_SESSION_INTERVAL;
> +import static org.jclouds.oauth.v2.config.OAuthProperties.RESOURCE;
> +
> +/**
> + * Authorizes new Bearer Tokens at runtime by sending up for the http request.
> + *
> + * <h3>Cache</h3>
> + * This maintains a time-based Bearer Token cache. By default expires after 59 minutes
> + * (the maximum time a token is valid is 60 minutes).
> + * This cache and expiry period is system-wide and does not attend to per-instance expiry time
> + * (e.g. "expires_in" from Google Compute -- which is set to the standard 3600 seconds).
Fix the javadoc to not reference Google in this flow?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/f375a9fa56cf71f32d857e40f5b93b3e2ef9e9b2#r57816046
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Jim Spring <no...@github.com>.
@nacx thanks for the feedback. Recommendations integrated. Waiting on build to complete, feedback integrated. I did have to make "audience" injectable for JWTBearerTokenFlow in the same manner I had to do for "resource" in ClientCredentialsSecretFlow.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941#issuecomment-203621462
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Jim Spring <no...@github.com>.
> + */
> + public static final String AZURE_CREDENTIAL = "oauth.azure-credential";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, an "azure-identity"
> + * is the GUID associated with an application given permssion to perform operations in
> + * Azure Resource Manager. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_IDENTITY = "oauth.azure-identity";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, the azure-endpoint
> + * is the URL https://login.microsoftonline.com/<Tenant ID>/oauth2/token, where <Tenant ID>
> + * corresponds to your Azure AD Tenant. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_ENDPOINT = "oauth.azure-endpoint";
That makes sense. I wasn't sure if you wanted live tests to cover each auth type independently. This makes it easy to fold some things in, etc.
On another note, "client_credentials" refers to the "grant_type" in the OAuth request. Secret because we currently use Secret. There is another flow with Azure AD where the grant_type is still "client_credentials", but it uses a certificate flow that differs from how it is done in the current Google path.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/f375a9fa56cf71f32d857e40f5b93b3e2ef9e9b2#r57820081
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
Closed #941.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941#event-611314692
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
Thanks for the contribution @jmspring!
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941#issuecomment-203155420
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> + public static final String AZURE_CREDENTIAL = "oauth.azure-credential";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, an "azure-identity"
> + * is the GUID associated with an application given permssion to perform operations in
> + * Azure Resource Manager. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_IDENTITY = "oauth.azure-identity";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, the azure-endpoint
> + * is the URL https://login.microsoftonline.com/<Tenant ID>/oauth2/token, where <Tenant ID>
> + * corresponds to your Azure AD Tenant. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_ENDPOINT = "oauth.azure-endpoint";
Perhaps I'm missing something here, but we just need to run the tests against one provider at a time. When running against Azure, users will set the azure credentials, endpoint *and the credential type property*. When testing against Google, they will set the Google credentials, and so when testing with DigitalOcean.
The key thing is to set the credential type, to let jclouds pick the right authentication flow. Setting it should allow to run the tests against the desired provider without issues... Unless I'm missing something? (It's been a while now since I played with the OAuth api internals)
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/f375a9fa56cf71f32d857e40f5b93b3e2ef9e9b2#r57817816
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
> + this.tokenDuration = tokenDuration;
> + // since the session interval is also the token expiration time requested to the server make the token expire a
> + // bit before the deadline to make sure there aren't session expiration exceptions
> + long cacheExpirationSeconds = tokenDuration > 30 ? tokenDuration - 30 : tokenDuration;
> + this.tokenCache = CacheBuilder.newBuilder().expireAfterWrite(cacheExpirationSeconds, SECONDS).build(loader);
> + }
> +
> + static final class AuthorizeToken extends CacheLoader<ClientSecret, Token> {
> + private final AuthorizationApi api;
> +
> + @Inject AuthorizeToken(AuthorizationApi api) {
> + this.api = api;
> + }
> +
> + @Override public Token load(ClientSecret key) throws Exception {
> + return api.authorizeClientSecret(key.clientId(), key.clientSecret(), key.resource());
According to [the spec](http://tools.ietf.org/html/rfc6749#section-4.4.2), the scope parameter is optional. It is already present in the `ClientSecret` object and we're configuring it in the Azure OAuth module. Is there any reason why it is not being sent in the request?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/f375a9fa56cf71f32d857e40f5b93b3e2ef9e9b2#r57815820
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -50,7 +50,7 @@
> public class JWTBearerTokenFlow implements OAuthFilter {
> private static final Joiner ON_COMMA = Joiner.on(",");
>
> - private final String audience;
> + @com.google.inject.Inject(optional = true) @Named(AUDIENCE) private String audience;
This is marked optional to make the injector happy when other flows don't need this, but I'd add a `checkNotNull` validation in the `filter` method so we provide a proper feedback at runtime if this value is missing.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/4bb749e1a41d5818a8c5e327b92096dfe984e0b4#r58028203
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Jim Spring <no...@github.com>.
@nacx -- Updates pushed. One note - regarding "scopes" for Azure, I made it an optional injection, per your feedback. For interactions with Azure Resource Manager APIs, scope, at least for the Credential/Password path, is not used. There are scenarios where it appears to be, but I have not investigated all of them.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941#issuecomment-204020471
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Jim Spring <no...@github.com>.
I'm not seeing the failures in oauth core, which is where the changes are. The failure seems to be in openstack.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941#issuecomment-203696036
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
BTW, tests that depend on OAuth seem to be failing because they can't build the context because they're missing the `jclouds.oauth.resource` property:
No implementation for java.lang.String annotated with
@com.google.inject.name.Named(value=jclouds.oauth.resource)
This is because they depend on the OAuthModule and it declares the client credentials flow, which requires that property. I'd say the most straightforward option is to configure the property to be optional in the client flow by moving it outside the constructor and annotating the variable with `@Inject(optional = true)`.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941#issuecomment-203155327
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
>I'm not seeing the failures in oauth core, which is where the changes are. The failure seems to be in openstack.
You're right; just ignore it. Looks like an internal failure in the Jenkins instance that runs the builds.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941#issuecomment-203858819
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
> + }
> +
> + /** OAuth isn't registered as a provider intentionally, so we fake one. */
> + @Override protected ProviderMetadata createProviderMetadata() {
> + return forApiOnEndpoint(AuthorizationApi.class, endpoint).toBuilder().id("oauth").build();
> + }
> +
> + @Override protected Properties setupProperties() {
> + Properties props = super.setupProperties();
> + props.put(CREDENTIAL_TYPE, CredentialType.CLIENT_CREDENTIALS_SECRET.toString());
> + resource = "https://management.azure.com/";
> + props.put(RESOURCE, resource);
> + endpoint = checkNotNull(setIfTestSystemPropertyPresent(props, OAuthProperties.AZURE_ENDPOINT), OAuthProperties.AZURE_ENDPOINT);
> + identity = checkNotNull(setIfTestSystemPropertyPresent(props, OAuthProperties.AZURE_IDENTITY), OAuthProperties.AZURE_IDENTITY);
> + credential = checkNotNull(setIfTestSystemPropertyPresent(props, OAuthProperties.AZURE_CREDENTIAL), OAuthProperties.AZURE_CREDENTIAL);
> + audience = checkNotNull(setIfTestSystemPropertyPresent(props, OAuthProperties.AUDIENCE), "test.jclouds.oauth.audience");
Remove this variable. It is not used in the client credentials flow.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/f375a9fa56cf71f32d857e40f5b93b3e2ef9e9b2#r57816226
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Jim Spring <no...@github.com>.
> + */
> + public static final String AZURE_CREDENTIAL = "oauth.azure-credential";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, an "azure-identity"
> + * is the GUID associated with an application given permssion to perform operations in
> + * Azure Resource Manager. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_IDENTITY = "oauth.azure-identity";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, the azure-endpoint
> + * is the URL https://login.microsoftonline.com/<Tenant ID>/oauth2/token, where <Tenant ID>
> + * corresponds to your Azure AD Tenant. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_ENDPOINT = "oauth.azure-endpoint";
One more question, re: live tests, if the credential type specified isn't relevant to a particular test, say JWT token, we can do this in one of two ways --
- single function that tests based off of credential type
- or, if the test isn't relevant to the credential type, just assertTrue(credentialType != credentialTypeRelevantToTest) in an if/then?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/f375a9fa56cf71f32d857e40f5b93b3e2ef9e9b2#r57821349
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> + public static final String AZURE_CREDENTIAL = "oauth.azure-credential";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, an "azure-identity"
> + * is the GUID associated with an application given permssion to perform operations in
> + * Azure Resource Manager. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_IDENTITY = "oauth.azure-identity";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, the azure-endpoint
> + * is the URL https://login.microsoftonline.com/<Tenant ID>/oauth2/token, where <Tenant ID>
> + * corresponds to your Azure AD Tenant. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_ENDPOINT = "oauth.azure-endpoint";
Got it. That's fine as long as we can qualify the credential type in a wat that allows jclouds to pick the right flow. So it should be ok if at some point we need to add that flow too.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/f375a9fa56cf71f32d857e40f5b93b3e2ef9e9b2#r57820500
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
> regarding "scopes" for Azure, I made it an optional injection, per your feedback
That sounds good. Making it optional still allows the Azure provider to configure, in a future, custom scopes for the requests that need them.
So just one last minor comment about a potential NPE, and this is good to go! Great job here @jmspring!
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941#issuecomment-204305645
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> + public static final String AZURE_CREDENTIAL = "oauth.azure-credential";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, an "azure-identity"
> + * is the GUID associated with an application given permssion to perform operations in
> + * Azure Resource Manager. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_IDENTITY = "oauth.azure-identity";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, the azure-endpoint
> + * is the URL https://login.microsoftonline.com/<Tenant ID>/oauth2/token, where <Tenant ID>
> + * corresponds to your Azure AD Tenant. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_ENDPOINT = "oauth.azure-endpoint";
That would be clean, but in the test report we wouldn't have immediate feedback on which auth method was actually tested.
WDYT about this: create a `dataProvider` like we do in the filesystem api integration tests (see [here](https://github.com/jclouds/jclouds/blob/master/apis/filesystem/src/test/java/org/jclouds/filesystem/integration/FilesystemContainerIntegrationTest.java#L89) and [here](https://github.com/jclouds/jclouds/blob/master/apis/filesystem/src/test/java/org/jclouds/filesystem/integration/FilesystemContainerIntegrationTest.java#L166-L170)) that executes the test only if the credential type is the right one. It is pretty much the same than throwing an `SkipException` if the credential type is not the expected one, but I find this approach cleaner and the test annotation would already provide a clear statement on when it is executed.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/f375a9fa56cf71f32d857e40f5b93b3e2ef9e9b2#r57850559
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
Pushed to master as [04207416](http://git-wip-us.apache.org/repos/asf/jclouds/commit/04207416). Thanks @jmspring!
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941#issuecomment-204409835
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Jim Spring <no...@github.com>.
Done. Thanks for your feedback.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941#issuecomment-204381791
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> + public static final String AZURE_CREDENTIAL = "oauth.azure-credential";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, an "azure-identity"
> + * is the GUID associated with an application given permssion to perform operations in
> + * Azure Resource Manager. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_IDENTITY = "oauth.azure-identity";
> +
> + /**
> + * When using oauth with Azure Active Direction and Client Credentials, the azure-endpoint
> + * is the URL https://login.microsoftonline.com/<Tenant ID>/oauth2/token, where <Tenant ID>
> + * corresponds to your Azure AD Tenant. Only used for OAuth Live Tests.
> + */
> + public static final String AZURE_ENDPOINT = "oauth.azure-endpoint";
I mean, to run tests against Google one could do:
```bash
mvn clean install -Plive \
-Dtest.oauth.identity=<email addr> \
-Dtest.oauth.credential=<private key> \
-Dtest.oauth.endpoint=https://accounts.google.com/o/oauth2/token \
-Dtest.jclouds.oauth.audience=https://accounts.google.com/o/oauth2/token \
-Dtest.jclouds.oauth.scope=https://www.googleapis.com/auth/prediction \
```
And to run them against Azure:
```bash
mvn clean install -Plive \
-Dtest.oauth.identity=<azure app id> \
-Dtest.oauth.credential=<azure app password> \
-Dtest.oauth.endpoint=https://login.microsoftonline.com/<tenant id>/oauth2/token \
-Djclouds.oauth.resource=https://management.azure.com/ \
-Djclouds.oauth.credential-type=clientCredentialsSecret
```
Should that work?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/f375a9fa56cf71f32d857e40f5b93b3e2ef9e9b2#r57818264
Re: [jclouds/jclouds] Add support for Azure AD authentication using
Service Principal (#941)
Posted by Ignasi Barrera <no...@github.com>.
> + @Inject AuthorizeToken(AuthorizationApi api) {
> + this.api = api;
> + }
> +
> + @Override public Token load(ClientSecret key) throws Exception {
> + return api.authorizeClientSecret(key.clientId(), key.clientSecret(), key.resource(), key.scope());
> + }
> + }
> +
> + @Override public HttpRequest filter(HttpRequest request) throws HttpException {
> + long now = currentTimeSeconds();
> + ClientSecret client = ClientSecret.create(
> + credentialsSupplier.get().identity,
> + credentialsSupplier.get().credential,
> + resource == null ? "" : resource,
> + ON_SPACE.join(scopes.forRequest(request)),
Take care of the NPE here. The scopes variable likely to be `null` in Azure.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/941/files/b3f576f0a5d781596fe0788c29d00c2d71a17037#r58175511