You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Ignasi Barrera <no...@github.com> on 2013/08/05 01:14:13 UTC
[jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Properly implemented token based authentication. An initial request is performed using Basic Authentication to request an authentication token. Subsequent requests will use the obtained token to authenticate, and once it expires a new one will be automatically requested.
This changes were inspired in Keystone and OpenStack common authentication.
/cc @carlosgarciaibanez
You can merge this Pull Request by running:
git pull https://github.com/nacx/jclouds-labs 223-tokenauth
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs/pull/20
-- Commit Summary --
* JCLOUDS-223: Implement token based auth in Abiquo
-- File Changes --
M abiquo/src/main/java/org/jclouds/abiquo/AbiquoApiMetadata.java (8)
A abiquo/src/main/java/org/jclouds/abiquo/config/AbiquoAuthenticationModule.java (93)
M abiquo/src/main/java/org/jclouds/abiquo/config/AbiquoProperties.java (15)
A abiquo/src/main/java/org/jclouds/abiquo/config/Authentication.java (36)
A abiquo/src/main/java/org/jclouds/abiquo/functions/auth/GetTokenFromApi.java (113)
A abiquo/src/main/java/org/jclouds/abiquo/functions/auth/GetTokenFromCredentials.java (44)
M abiquo/src/main/java/org/jclouds/abiquo/http/filters/AbiquoAuthentication.java (35)
M abiquo/src/test/java/org/jclouds/abiquo/features/BaseAbiquoApiExpectTest.java (32)
M abiquo/src/test/java/org/jclouds/abiquo/features/CloudApiExpectTest.java (4)
M abiquo/src/test/java/org/jclouds/abiquo/features/InfrastructureApiExpectTest.java (12)
M abiquo/src/test/java/org/jclouds/abiquo/features/VirtualMachineTemplateApiExpectTest.java (2)
A abiquo/src/test/java/org/jclouds/abiquo/functions/auth/GetTokenFromApiTest.java (91)
D abiquo/src/test/java/org/jclouds/abiquo/http/filters/AbiquoAuthenticationLiveApiTest.java (174)
M abiquo/src/test/java/org/jclouds/abiquo/http/filters/AbiquoAuthenticationTest.java (50)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/20.patch
https://github.com/jclouds/jclouds-labs/pull/20.diff
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Andrew Phillips <no...@github.com>.
> + protected Logger logger = Logger.NULL;
> +
> + @Inject
> + public GetTokenFromApi(ProviderMetadata provider, HttpClient http) {
> + this.provider = checkNotNull(provider, "provider must not be null");
> + this.http = checkNotNull(http, "http must not be null");
> + }
> +
> + @Override
> + public String apply(Credentials input) {
> + logger.info(">> Requesting an authentication token for user: %s...", input.identity);
> +
> + HttpResponse response = http.invoke(HttpRequest.builder() //
> + .method("GET") //
> + .endpoint(URI.create(provider.getEndpoint())) //
> + .addHeader(AUTHORIZATION, basic(input.identity, input.credential)) //
Ah, interesting trick!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20/files#r5655632
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Andrew Phillips <no...@github.com>.
Thanks for explaining, @nacx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20#issuecomment-22174025
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Ignasi Barrera <no...@github.com>.
> + protected Logger logger = Logger.NULL;
> +
> + @Inject
> + public GetTokenFromApi(ProviderMetadata provider, HttpClient http) {
> + this.provider = checkNotNull(provider, "provider must not be null");
> + this.http = checkNotNull(http, "http must not be null");
> + }
> +
> + @Override
> + public String apply(Credentials input) {
> + logger.info(">> Requesting an authentication token for user: %s...", input.identity);
> +
> + HttpResponse response = http.invoke(HttpRequest.builder() //
> + .method("GET") //
> + .endpoint(URI.create(provider.getEndpoint())) //
> + .addHeader(AUTHORIZATION, basic(input.identity, input.credential)) //
They are there just to keep the line breaks, as most IDE formatters would
put all those in one. Just readability!
El 08/08/2013 07:36, "Andrew Phillips" <no...@github.com> escribió:
> In
> abiquo/src/main/java/org/jclouds/abiquo/functions/auth/GetTokenFromApi.java:
>
> > + protected Logger logger = Logger.NULL;
> > +
> > + @Inject
> > + public GetTokenFromApi(ProviderMetadata provider, HttpClient http) {
> > + this.provider = checkNotNull(provider, "provider must not be null");
> > + this.http = checkNotNull(http, "http must not be null");
> > + }
> > +
> > + @Override
> > + public String apply(Credentials input) {
> > + logger.info(">> Requesting an authentication token for user: %s...", input.identity);
> > +
> > + HttpResponse response = http.invoke(HttpRequest.builder() //
> > + .method("GET") //
> > + .endpoint(URI.create(provider.getEndpoint())) //
> > + .addHeader(AUTHORIZATION, basic(input.identity, input.credential)) //
>
> Just out of curiosity...what are the // bits doing here? Something left
> over from commenting out bits of code during development?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/jclouds/jclouds-labs/pull/20/files#r5651011>
> .
>
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20/files#r5651428
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Ignasi Barrera <no...@github.com>.
Closed #20.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Andrew Phillips <no...@github.com>.
> + this.http = checkNotNull(http, "http must not be null");
> + }
> +
> + @Override
> + public String apply(Credentials input) {
> + logger.info(">> Requesting an authentication token for user: %s...", input.identity);
> +
> + HttpResponse response = http.invoke(HttpRequest.builder() //
> + .method("GET") //
> + .endpoint(URI.create(provider.getEndpoint())) //
> + .addHeader(AUTHORIZATION, basic(input.identity, input.credential)) //
> + .build());
> +
> + Optional<Cookie> token = readAuthenticationToken(response);
> + if (!token.isPresent()) {
> + throw new AuthorizationException();
Message here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20/files#r5596064
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Ignasi Barrera <no...@github.com>.
> + }
> +
> + @Provides
> + @Singleton
> + protected Function<Credentials, String> authenticationMethodForCredentialType(
> + Map<String, Function<Credentials, String>> authenticationMethods, @Named(CREDENTIAL_TYPE) String credentialType) {
> + checkArgument(authenticationMethods.containsKey(credentialType), "credential type %s not in supported list: %s",
> + credentialType, authenticationMethods.keySet());
> + return authenticationMethods.get(credentialType);
> + }
> +
> + // Abiquo authentication tokens have 30 minutes life time
> + @Provides
> + @Singleton
> + protected LoadingCache<Credentials, String> provideTokenCache(Function<Credentials, String> getToken) {
> + return CacheBuilder.newBuilder().expireAfterWrite(30, TimeUnit.MINUTES).build(CacheLoader.from(getToken));
Yes, it might be better to renew a bit before just to be sure.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20/files#r5598541
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Andrew Phillips <no...@github.com>.
> + protected Logger logger = Logger.NULL;
> +
> + @Inject
> + public GetTokenFromApi(ProviderMetadata provider, HttpClient http) {
> + this.provider = checkNotNull(provider, "provider must not be null");
> + this.http = checkNotNull(http, "http must not be null");
> + }
> +
> + @Override
> + public String apply(Credentials input) {
> + logger.info(">> Requesting an authentication token for user: %s...", input.identity);
> +
> + HttpResponse response = http.invoke(HttpRequest.builder() //
> + .method("GET") //
> + .endpoint(URI.create(provider.getEndpoint())) //
> + .addHeader(AUTHORIZATION, basic(input.identity, input.credential)) //
Just out of curiosity...what are the `//` bits doing here? Something left over from commenting out bits of code during development?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20/files#r5651011
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Ignasi Barrera <no...@github.com>.
There is no "old" authentication right now. Before this PR, there were 2 ways to authenticate the API calls: using Basic Auth (in each request) or using an authentication token, and that depended on how the context was created. With this PR, either you create it using a username and password or an existing token, the authentication will always be token based. In the first case, an initial request will be made to ask for an authentication token; in the latter, the provided token will be used.
This second case is common for users integrating the API in their own apps and portals. They may have already generated the token by other means (not using jclouds), so I'll just remove the deprecation of the `GetTokenFromCredentials`, as we should keep it as a valid way to create the context.
So, with this PR there is only one way to authenticate against the API right now, regardless of how users create the context, so in practice, each live test is implicitly testing it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20#issuecomment-22164523
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Ignasi Barrera <no...@github.com>.
Addressed comments. Thanks @demobox!
The `AuthorizationException` change was made in https://github.com/jclouds/jclouds/pull/101
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20#issuecomment-22241682
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Andrew Phillips <no...@github.com>.
> + }
> +
> + @Provides
> + @Singleton
> + protected Function<Credentials, String> authenticationMethodForCredentialType(
> + Map<String, Function<Credentials, String>> authenticationMethods, @Named(CREDENTIAL_TYPE) String credentialType) {
> + checkArgument(authenticationMethods.containsKey(credentialType), "credential type %s not in supported list: %s",
> + credentialType, authenticationMethods.keySet());
> + return authenticationMethods.get(credentialType);
> + }
> +
> + // Abiquo authentication tokens have 30 minutes life time
> + @Provides
> + @Singleton
> + protected LoadingCache<Credentials, String> provideTokenCache(Function<Credentials, String> getToken) {
> + return CacheBuilder.newBuilder().expireAfterWrite(30, TimeUnit.MINUTES).build(CacheLoader.from(getToken));
Expire a bit earlier, e.g. after 29min? That way the risk (small as it is) of an expired token being sent is avoided?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20/files#r5596034
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Ignasi Barrera <no...@github.com>.
Merged!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20#issuecomment-22286948
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Andrew Phillips <no...@github.com>.
They're probably there, so just a quick question:
* are there any tests that still check the old behaviour?
* should any kind of deprecation warning be displayed in the logs if users use the old form of authentication?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20#issuecomment-22156383
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Ignasi Barrera <no...@github.com>.
> + this.http = checkNotNull(http, "http must not be null");
> + }
> +
> + @Override
> + public String apply(Credentials input) {
> + logger.info(">> Requesting an authentication token for user: %s...", input.identity);
> +
> + HttpResponse response = http.invoke(HttpRequest.builder() //
> + .method("GET") //
> + .endpoint(URI.create(provider.getEndpoint())) //
> + .addHeader(AUTHORIZATION, basic(input.identity, input.credential)) //
> + .build());
> +
> + Optional<Cookie> token = readAuthenticationToken(response);
> + if (!token.isPresent()) {
> + throw new AuthorizationException();
Yes. I don't know why there was no constructor accepting only a message (all require a Throwable too), but it seems to make sense. I'll add the constructor (in the jclouds repo) and then update this PR.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20/files#r5598634
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #268](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/268/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20#issuecomment-22241957
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Andrew Phillips <no...@github.com>.
> @@ -16,6 +16,7 @@
> */
> package org.jclouds.abiquo.config;
>
> +
Intentional?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20/files#r5596052
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #32](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/32/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20#issuecomment-22081242
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #253](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/253/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20#issuecomment-22081209
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #34](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/34/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20#issuecomment-22242087
Re: [jclouds-labs] JCLOUDS-223: Implement token based auth in Abiquo
(#20)
Posted by Andrew Phillips <no...@github.com>.
> + @Provides
> + @Singleton
> + @Authentication
> + protected Supplier<String> provideTokenSupplier(final LoadingCache<Credentials, String> cache,
> + @Provider final Supplier<Credentials> creds) {
> + return new Supplier<String>() {
> + @Override
> + public String get() {
> + return cache.getUnchecked(creds.get());
> + }
> + };
> + }
> +
> + @Override
> + protected void configure() {
> +
Add a comment such as `// override me`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/20/files#r5596049