You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Adrian Cole <no...@github.com> on 2014/11/18 04:54:11 UTC
[jclouds-labs-google] OAuth cleanup round... 5? (#96)
OAuth smoke and mirror indirection is coming to an end. Through tracing code, I've found that we only actually issue Bearer tokens. I've renamed accordingly, and flattened a complex cache mapping. There's at least one more cleanup round to do, but this should be the last api-breaker
TODO for implementors (aka @ccustine)
* Use OAuthModule, not OAuthAuthenticationModule
* Use OAuthFilter, not OAuthAuthenticationFilter
* else is the same
Details.
* Refactor OAuth to be less complex.
* Remove OAuth as a registered api.
* Rename redundant OAuthAuthenticationFilter to OAuthFilter.
You can merge this Pull Request by running:
git pull https://github.com/adriancole/jclouds-labs-google adrian.oauth-cleanup-again
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-google/pull/96
-- Commit Summary --
* Refactor OAuth to be less complex. Remove oauth as a standalone api. Rename redundant OAuthAuthenticationFilter to OAuthFilter.
-- File Changes --
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/GoogleCloudStorageApiMetadata.java (2)
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/BucketAccessControlsApi.java (4)
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/BucketApi.java (4)
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/DefaultObjectAccessControlsApi.java (4)
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/ObjectAccessControlsApi.java (4)
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/ObjectApi.java (4)
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/ResumableUploadApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/GoogleComputeEngineApiMetadata.java (2)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/Resources.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/config/GoogleComputeEngineHttpApiModule.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/AddressApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/AggregatedListApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/DiskApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/DiskTypeApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/FirewallApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/ForwardingRuleApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/HttpHealthCheckApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/ImageApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/InstanceApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/MachineTypeApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/NetworkApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/OperationApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/ProjectApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/RegionApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/RouteApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/SnapshotApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/TargetPoolApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/ZoneApi.java (4)
M oauth/pom.xml (1)
R oauth/src/main/java/org/jclouds/oauth/v2/AuthorizationApi.java (37)
D oauth/src/main/java/org/jclouds/oauth/v2/OAuthApi.java (61)
D oauth/src/main/java/org/jclouds/oauth/v2/OAuthApiMetadata.java (83)
R oauth/src/main/java/org/jclouds/oauth/v2/config/Authorization.java (6)
D oauth/src/main/java/org/jclouds/oauth/v2/config/OAuthAuthenticationModule.java (46)
D oauth/src/main/java/org/jclouds/oauth/v2/config/OAuthHttpApiModule.java (42)
M oauth/src/main/java/org/jclouds/oauth/v2/config/OAuthModule.java (48)
R oauth/src/main/java/org/jclouds/oauth/v2/filters/BearerTokenFromCredentials.java (8)
A oauth/src/main/java/org/jclouds/oauth/v2/filters/DynamicBearerToken.java (74)
D oauth/src/main/java/org/jclouds/oauth/v2/filters/OAuthAuthenticator.java (51)
R oauth/src/main/java/org/jclouds/oauth/v2/filters/OAuthFilter.java (2)
M oauth/src/main/java/org/jclouds/oauth/v2/functions/PrivateKeySupplier.java (2)
M oauth/src/main/java/org/jclouds/oauth/v2/functions/SignOrProduceMacForToken.java (4)
D oauth/src/main/java/org/jclouds/oauth/v2/handlers/OAuthErrorHandler.java (58)
A oauth/src/test/java/org/jclouds/oauth/v2/AuthorizationApiLiveTest.java (102)
R oauth/src/test/java/org/jclouds/oauth/v2/AuthorizationApiMockTest.java (31)
D oauth/src/test/java/org/jclouds/oauth/v2/OAuthApiMetadataTest.java (39)
D oauth/src/test/java/org/jclouds/oauth/v2/features/OAuthApiLiveTest.java (84)
D oauth/src/test/java/org/jclouds/oauth/v2/handlers/OAuthErrorHandlerTest.java (92)
D oauth/src/test/java/org/jclouds/oauth/v2/internal/Base64UrlSafeTest.java (40)
D oauth/src/test/java/org/jclouds/oauth/v2/internal/BaseOAuthApiLiveTest.java (63)
D oauth/src/test/java/org/jclouds/oauth/v2/internal/BaseOAuthAuthenticatedApiLiveTest.java (112)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-google/pull/96.patch
https://github.com/jclouds/jclouds-labs-google/pull/96.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Andrew Phillips <no...@github.com>.
> +import org.jclouds.rest.AuthorizationException;
> +
> +import com.google.common.base.Function;
> +import com.google.common.base.Supplier;
> +import com.google.common.collect.ImmutableList;
> +
> +public final class ClaimsToAssertion implements Function<Object, String> {
> + private static final List<String> SUPPORTED_ALGS = ImmutableList.of("RS256", "none");
> +
> + private final Supplier<PrivateKey> privateKey;
> + private final Json json;
> + private final String alg;
> +
> + @Inject ClaimsToAssertion(@Named(JWS_ALG) String alg, @Authorization Supplier<PrivateKey> privateKey, Json json) {
> + this.alg = alg;
> + checkArgument(SUPPORTED_ALGS.contains(alg), "%s %s not in supported list", JWS_ALG, alg, SUPPORTED_ALGS);
Two `%s` and three args?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96/files#r20546410
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Adrian Cole <no...@github.com>.
and now.. the 1k less lines edition! @ccustine @nacx ptal
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#issuecomment-63426892
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1746](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1746/) 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-google/pull/96#issuecomment-63428629
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1745](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1745/) 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-google/pull/96#issuecomment-63427771
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #302](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/302/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#issuecomment-63428398
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Adrian Cole <no...@github.com>.
> +import org.jclouds.rest.AuthorizationException;
> +
> +import com.google.common.base.Function;
> +import com.google.common.base.Supplier;
> +import com.google.common.collect.ImmutableList;
> +
> +public final class ClaimsToAssertion implements Function<Object, String> {
> + private static final List<String> SUPPORTED_ALGS = ImmutableList.of("RS256", "none");
> +
> + private final Supplier<PrivateKey> privateKey;
> + private final Json json;
> + private final String alg;
> +
> + @Inject ClaimsToAssertion(@Named(JWS_ALG) String alg, @Authorization Supplier<PrivateKey> privateKey, Json json) {
> + this.alg = alg;
> + checkArgument(SUPPORTED_ALGS.contains(alg), "%s %s not in supported list", JWS_ALG, alg, SUPPORTED_ALGS);
thx. will fix on next round
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96/files#r20553080
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1744](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1744/) 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-google/pull/96#issuecomment-63427100
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #300](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/300/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#issuecomment-63426957
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Andrea Turli <no...@github.com>.
don't have a way to run LiveTests, can you report the result from your tests?
If they are ok, I think the assumptions done to simplify the implementation are ok for Google's product. If we then want to support OAuth for other providers will probably need to revisit that again, correct?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#issuecomment-63507998
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Andrew Phillips <no...@github.com>.
> - expirationSeconds = expirationSeconds > 30 ? expirationSeconds - 30 : expirationSeconds;
> - return CacheBuilder.newBuilder().expireAfterWrite(expirationSeconds, SECONDS).build(CacheLoader.from(getAccess));
> - }
> -
> - /**
> - * Defers instantiation of {@linkplain SignOrProduceMacForToken} so as to avoid requiring private keys when the alg
> - * is set to {@linkplain org.jclouds.oauth.v2.JWSAlgorithms#NONE}.
> - */
> - @Provides @Singleton Supplier<Function<byte[], byte[]>> signOrProduceMacForToken(@Named(JWS_ALG) String jwsAlg,
> - Provider<SignOrProduceMacForToken> in) {
> - if (jwsAlg.equals(NONE)) { // Current implementation requires we return null on none.
> - return (Supplier) Suppliers.ofInstance(Functions.constant(null));
> - }
> - return Suppliers.memoize(in.get());
> + @Authorization
> + protected Supplier<URI> oauthEndpoint(@javax.inject.Named("oauth.endpoint") String endpoint) {
> but seems I need to do a global sweep.
There's another one down here ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96/files#r20550173
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Adrian Cole <no...@github.com>.
> +
> + @Inject ClaimsToAssertion(@Named(JWS_ALG) String alg, @Authorization Supplier<PrivateKey> privateKey, Json json) {
> + this.alg = alg;
> + checkArgument(SUPPORTED_ALGS.contains(alg), "%s %s not in supported list", JWS_ALG, alg, SUPPORTED_ALGS);
> + this.privateKey = privateKey;
> + this.json = json;
> + }
> +
> + @Override public String apply(Object input) {
> + String encodedHeader = String.format("{\"alg\":\"%s\",\"typ\":\"JWT\"}", alg);
> + String encodedClaimSet = json.toJson(input);
> +
> + encodedHeader = base64Url().omitPadding().encode(encodedHeader.getBytes(UTF_8));
> + encodedClaimSet = base64Url().omitPadding().encode(encodedClaimSet.getBytes(UTF_8));
> +
> + byte[] signature = alg.equals("none")
much simpler than the weird `Function<byte[], byte[]>` indirection, right?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96/files#r20487253
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Adrian Cole <no...@github.com>.
Simplifications are made around JWT actually, not necessarily google. For
example, I used salesforce docs to clarify terminology!
https://help.salesforce.com/HTViewHelpDoc?id=remoteaccess_oauth_jwt_flow.htm
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#issuecomment-63508451
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Adrian Cole <no...@github.com>.
> @Provides
> - @Singleton
> - public LoadingCache<TokenRequest, Token> provideAccessCache(Function<TokenRequest, Token> getAccess,
> - @Named(PROPERTY_SESSION_INTERVAL) long expirationSeconds) {
> - // 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
> - expirationSeconds = expirationSeconds > 30 ? expirationSeconds - 30 : expirationSeconds;
Google projects were using mixed versions for some historical accident.
I've been cleaning as I went along, but seems I need to do a global sweep.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96/files#r20533744
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Adrian Cole <no...@github.com>.
and.. in
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#issuecomment-63522384
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Adrian Cole <no...@github.com>.
GCE tests were doing fine, then I screwed up my classpath working on something unrelated about 30m in. I'm going to merge, and certainly run live tests again on next change
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#issuecomment-63522120
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Andrea Turli <no...@github.com>.
I'd like to see the LiveTests result to +1 unless @nacx @ccustine have other thoughts
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#issuecomment-63509637
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Andrew Phillips <no...@github.com>.
Just a couple of minor comments. +1 - looks good to me. Thanks, @adriancole!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#issuecomment-63563273
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #299](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/299/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#issuecomment-63422931
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Andrea Turli <no...@github.com>.
> @@ -35,7 +35,7 @@
> import org.jclouds.googlecloudstorage.domain.templates.BucketAccessControlsTemplate;
> import org.jclouds.http.HttpResponse;
> import org.jclouds.javax.annotation.Nullable;
> -import org.jclouds.oauth.v2.filters.OAuthAuthenticationFilter;
> +import org.jclouds.oauth.v2.filters.OAuthFilter;
renaming looks sensible
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96/files#r20520841
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #298](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/298/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#issuecomment-63419376
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Adrian Cole <no...@github.com>.
Live test from oauth passed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#issuecomment-63513431
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Andrew Phillips <no...@github.com>.
> @Provides
> - @Singleton
> - public LoadingCache<TokenRequest, Token> provideAccessCache(Function<TokenRequest, Token> getAccess,
> - @Named(PROPERTY_SESSION_INTERVAL) long expirationSeconds) {
> - // 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
> - expirationSeconds = expirationSeconds > 30 ? expirationSeconds - 30 : expirationSeconds;
[minor] Any reason we're using javax.inject.Named here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96/files#r20533522
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Adrian Cole <no...@github.com>.
similar error ratio on google storage
```
Tests run: 101, Failures: 7, Errors: 0, Skipped: 55
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#issuecomment-63514650
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1742](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1742/) 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-google/pull/96#issuecomment-63419548
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Adrian Cole <no...@github.com>.
>
> import com.google.common.base.Function;
> import com.google.common.base.Joiner;
> import com.google.common.base.Supplier;
>
> /** Builds the default token request with the following claims: {@code iss,scope,aud,iat,exp}. */
> -public class BuildTokenRequest implements Function<HttpRequest, TokenRequest> {
> +public class HttpRequestToClaims implements Function<HttpRequest, Claims> {
finally figured out what this class was doing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96/files#r20487267
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1743](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1743/) UNSTABLE
Looks like there's a problem with this pull request
[(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-google/pull/96#issuecomment-63423166
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #301](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/301/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#issuecomment-63427590
Re: [jclouds-labs-google] OAuth cleanup round... 5? (#96)
Posted by Adrian Cole <no...@github.com>.
Closed #96.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/96#event-195048816