You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by CARRIERE Etienne <no...@github.com> on 2017/01/03 17:19:04 UTC
[jclouds/jclouds] JCLOUDS-1220 : Managing the header name in the
TempAuth (Identity Protocol v1) (#1046)
State of jclouds
The openstack swift "official" client (in python) manage this v1 protocol (http://docs.openstack.org/developer/python-swiftclient/swiftclient.html) so even if we don't have a specification, we will use the code of the official client for the implementation.
In jclouds, there is currently a sort-of v1 identity protocol in the openstack-swift module: in apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/config/SwiftAuthenticationModule.java, there is a tempAuthCredentials which is almost the identity v1 protocol except that the name of the headers was not the same :
= X-Storage-User vs X-Auth-User
= X-Storage-Path vs X-Auth-Key
Pull request :
= Keep the current behaviour as default
= Add 2 parameters to change the header name through variables in the Properties put in the Builder like that :
Properties overrides = new Properties();
overrides.setProperty("jclouds.keystone.credential-type", "tempAuthCredentials");
overrides.setProperty("jclouds.swift.tempAuth.headerUser", "X-Auth-User");
overrides.setProperty("jclouds.swift.tempAuth.headerPass", "X-Auth-Pass");
swiftApi = ContextBuilder.newBuilder(provider)
.endpoint(args[1])
.credentials(identity, credential)
.modules(modules)
.overrides(overrides)
.buildApi(SwiftApi.class);
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/1046
-- Commit Summary --
* Possibility to change the name of Header of Identity v1 protocol
-- File Changes --
A apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/binders/TempAuthBinder.java (40)
M apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/config/SwiftAuthenticationModule.java (32)
M apis/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/TempAuthMockTest.java (26)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/1046.patch
https://github.com/jclouds/jclouds/pull/1046.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/1046
Re: [jclouds/jclouds] JCLOUDS-1220 : Managing the header name in the
TempAuth (Identity Protocol v1) (#1046)
Posted by Ignasi Barrera <no...@github.com>.
nacx requested changes on this pull request.
Thanks! Looks much better. Just a couple final comments!
> +
+import com.google.inject.Inject;
+import com.google.inject.name.Named;
+
+/**
+ * Binder to the tempAuthAuthentication
+ *
+ */
+public final class TempAuthBinder implements Binder{
+ @Inject
+ @Named(TEMP_AUTH_HEADER_USER)
+ private String identityHeaderNameUser;
+
+ @Inject
+ @Named(TEMP_AUTH_HEADER_PASS)
+ private String identityHeaderNamePass;
Declare the variables final and use [constructor injection](https://github.com/google/guice/wiki/Injections#constructor-injection).
> }
static final class AdaptTempAuthResponseToAccess
implements Function<HttpResponse, Access>, InvocationContext<AdaptTempAuthResponseToAccess> {
+ @Inject
+ @Named(TEMP_AUTH_HEADER_USER)
+ private String identityHeaderNameUser;
Add it to the constructor and declare it final to use constructor injection too.
> @@ -153,7 +156,7 @@ private URI getURI(String headerValue) {
public AdaptTempAuthResponseToAccess setContext(HttpRequest request) {
String host = request.getEndpoint().getHost();
this.host = host;
- this.username = request.getFirstHeaderOrNull(STORAGE_USER);
+ this.username = request.getFirstHeaderOrNull(identityHeaderNameUser);
[minor] Fix indentation
--
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/1046#pullrequestreview-15211240
Re: [jclouds/jclouds] JCLOUDS-1220 : Managing the header name in the
TempAuth (Identity Protocol v1) (#1046)
Posted by CARRIERE Etienne <no...@github.com>.
Compared to your proposal, I added a constant class in reference to regroup the constants.
--
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/1046#issuecomment-270483467
Re: [jclouds/jclouds] JCLOUDS-1220 : Managing the header name in the
TempAuth (Identity Protocol v1) (#1046)
Posted by Ignasi Barrera <no...@github.com>.
nacx requested changes on this pull request.
Thanks @Etienne-Carriere! The patch approach LGTM. I've added some suggestions to clean up the code a bit. Thanks!
> @@ -62,22 +64,40 @@
* in {@code auth/v1.0/}.
*/
public final class SwiftAuthenticationModule extends KeystoneAuthenticationModule {
- private static final String STORAGE_USER = "X-Storage-User";
- private static final String STORAGE_PASS = "X-Storage-Pass";
+ public static final String TEMP_AUTH_HEADER_USER = "jclouds.swift.tempAuth.headerUser";
+ public static final String TEMP_AUTH_HEADER_PASS = "jclouds.swift.tempAuth.headerPass";
+
+ @com.google.inject.Inject(optional = true)
+ @Named(TEMP_AUTH_HEADER_USER)
+ private static String identityHeaderNameUser = "X-Storage-User";
+
+ @com.google.inject.Inject(optional = true)
+ @Named(TEMP_AUTH_HEADER_PASS)
+ private static String identityHeaderNamePass = "X-Storage-Pass";
Move these variables to the `TempAuthBinder ` class.
> + */
+package org.jclouds.openstack.swift.v1.binders;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import org.jclouds.domain.Credentials;
+import org.jclouds.http.HttpRequest;
+import org.jclouds.openstack.swift.v1.config.SwiftAuthenticationModule;
+import org.jclouds.rest.Binder;
+
+/**
+ * Binder to the tempAuthAuthentication
+ *
+ */
+public final class TempAuthBinder implements Binder{
[style] Remove the extra spaces and add a space before the last `{`.
> private static final String STORAGE_URL = "X-Storage-Url";
@Override
protected void configure() {
super.configure();
bindHttpApi(binder(), AuthenticationApi.class);
bindHttpApi(binder(), TempAuthApi.class);
+ requestStaticInjection(SwiftAuthenticationModule.class);
Once the injected variables are not here, this should not be needed.
> }
@Override protected Map<String, Function<Credentials, Access>> authenticationMethods(Injector i) {
return ImmutableMap.<String, Function<Credentials, Access>>builder()
.putAll(super.authenticationMethods(i))
.put("tempAuthCredentials", i.getInstance(TempAuth.class)).build();
}
+
+ public static String getIdentityHeaderName(){
+ return identityHeaderNameUser;
+ }
+
+ public static String getIdentityHeaderPass(){
+ return identityHeaderNamePass;
+ }
These getters should no longer be needed.
> @@ -153,7 +173,7 @@ private URI getURI(String headerValue) {
public AdaptTempAuthResponseToAccess setContext(HttpRequest request) {
String host = request.getEndpoint().getHost();
this.host = host;
- this.username = request.getFirstHeaderOrNull(STORAGE_USER);
+ this.username = request.getFirstHeaderOrNull(SwiftAuthenticationModule.getIdentityHeaderName());
You can inject the header variable in this class too, to avoid dealing with static methods.
> @@ -45,13 +46,24 @@
public void testTempAuthRequest() throws Exception {
- tempAuthServer.enqueue(new MockResponse().setResponseCode(204)
+ Properties overrides = null;
+ // with default values
+ test(overrides);
+ overrides = new Properties();
+ overrides.setProperty(SwiftAuthenticationModule.TEMP_AUTH_HEADER_USER , "X-Auth-User");
+ overrides.setProperty(SwiftAuthenticationModule.TEMP_AUTH_HEADER_PASS , "X-Auth-Pass");
+ // with specific Header Name values
+ test(overrides);
Better move this to another test named `testTempAuthRequestWithCustomHeaders` or similar, to properly qualify and isolate the test that verifies this change.
> @@ -71,8 +83,10 @@ public void testTempAuthRequest() throws Exception {
assertEquals(listContainers.getHeader("X-Auth-Token"), "token");
}
- private SwiftApi api(String authUrl) throws IOException {
- Properties overrides = new Properties();
+ private SwiftApi api(String authUrl, Properties overrides) throws IOException {
+ if (overrides == null){
+ overrides = new Properties();
+ }
Instead of passing null, remove this check and call this method with a new Properties object.
> @@ -60,8 +72,8 @@ public void testTempAuthRequest() throws Exception {
RecordedRequest auth = tempAuthServer.takeRequest();
assertEquals(auth.getMethod(), "GET");
- assertEquals(auth.getHeader("X-Storage-User"), "user");
- assertEquals(auth.getHeader("X-Storage-Pass"), "password");
+ assertEquals(auth.getHeader(SwiftAuthenticationModule.getIdentityHeaderName()), "user");
+ assertEquals(auth.getHeader(SwiftAuthenticationModule.getIdentityHeaderPass()), "password");
Once there are two tests, it's better to keep the expected header name hardcoded, to properly verify that the expected configuration is applied.
--
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/1046#pullrequestreview-15037991
Re: [jclouds/jclouds] JCLOUDS-1220 : Managing the header name in the
TempAuth (Identity Protocol v1) (#1046)
Posted by CARRIERE Etienne <no...@github.com>.
@Etienne-Carriere pushed 1 commit.
a3c29a1 Modification to comply with jclouds coding style
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1046/files/df1bfd0ee038549e63fee7ec093f6a6b9f49b419..a3c29a17ffeb1a440c4a7585eef5bdb217a006bf
Re: [jclouds/jclouds] JCLOUDS-1220 : Managing the header name in the
TempAuth (Identity Protocol v1) (#1046)
Posted by CARRIERE Etienne <no...@github.com>.
@Etienne-Carriere pushed 1 commit.
f658c93 Use Constructor Injection and correct indentation
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1046/files/f27bd00db3de99b60024305b605917fd5771a331..f658c937b4ab5e74611ccf4a86afdd2f5a1fe656
Re: [jclouds/jclouds] JCLOUDS-1220 : Managing the header name in the
TempAuth (Identity Protocol v1) (#1046)
Posted by Ignasi Barrera <no...@github.com>.
Also, you can easily avoid the need for optional injection (so you can declare the variables final and use constructor injection) by setting the default values for the headers in the [SwiftApiMetadata default properties](https://github.com/jclouds/jclouds/blob/master/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/SwiftApiMetadata.java#L59-L66). That would be also a cleaner approach.
--
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/1046#issuecomment-270262882
Re: [jclouds/jclouds] JCLOUDS-1220 : Managing the header name in the
TempAuth (Identity Protocol v1) (#1046)
Posted by Ignasi Barrera <no...@github.com>.
Squashed the commits and amended the message to include the issue id.
Merged to [master](http://git-wip-us.apache.org/repos/asf/jclouds/commit/ac2f746e) and [2.0.x](http://git-wip-us.apache.org/repos/asf/jclouds/commit/01d74969). Thanks @Etienne-Carriere!
--
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/1046#issuecomment-270672185
Re: [jclouds/jclouds] JCLOUDS-1220 : Managing the header name in the
TempAuth (Identity Protocol v1) (#1046)
Posted by Ignasi Barrera <no...@github.com>.
Closed #1046.
--
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/1046#event-912796277