identity: use AuthOptionsBuilder for v2 auth

[cardoe]

Rather than creating a tokens2.AuthOptions inside the authentication flow for identity v2, allow any structure that implements the tokens2.AuthBuilderOptions interface to be used. The interface is modified to add a CanReauth function like the v3 version has but provides a default implementation. The gophercloud.AuthOptions already implements this interface so there is no change to its API. Lastly add an AuthenticateV2Ext function which will allow an out of tree identity v2 auth mechanism to be implemented. fixes #2651. ref #1330.

[cardoe]

I tried my best not to make an API change here. The unfortunate result is that tokens2.AuthOptionsBuilder does have a function added that mirrors what tokens3.AuthOptionsBuilder has. So anyone that implemented their own would need to add that method but it'd literally be a 1 liner.

I do think that my approach to handling the setting of AllowReauth when creating the ReauthFunc is something that the v3 side of the code could use and then out of tree auth plugins could be used for v3 as well. But I'm happy to do that in a separate PR.

[coveralls]

coverage: 78.277% (-0.04%) from 78.32%
when pulling 894f777 on cardoe:fix-auth-v2
into 280727e on gophercloud:master.

[cardoe]

Just wondering if you've had any thoughts on this approach @pierreprinetti ?

[EmilienM]

This LGTM but I'll like to see Pierre's review on this one.

[cardoe]

I noticed @mandre marked this as a major semver bump. If that's the case and I'll have to wait for 2.0 for this then would it make more sense to not add a method and instead change the existing one?

[cardoe]

Hoping you've got some feedback here @pierreprinetti

[pierreprinetti]

This change looks beautifully done. I have just a couple questions inline.

We will have to test the hell out of this PR when it's ready to merge.

[cardoe]

I've opened #3030 as well which performs the same changes in a non semver:major way.

[pierreprinetti]

Thank you for this, and sorry for the time it's taken to get a review.

I am not convinced that we need a type assertion, and I would only use that as a last resort. For example: how do you prevent an infinite reauth loop with custom types?

One alternative could be embedding and overloading the CanReauth method. Can you please see if this makes sense?

diff --git a/openstack/client.go b/openstack/client.go
index dfe5e53e..07cc6d40 100644
--- a/openstack/client.go
+++ b/openstack/client.go
@@ -113,6 +113,12 @@ func AuthenticateV2(ctx context.Context, client *gophercloud.ProviderClient, opt
 	return v2auth(ctx, client, "", options, eo)
 }
 
+type v2TokenNoReauth struct {
+	tokens2.AuthOptionsBuilder
+}
+
+func (v2TokenNoReauth) CanReauth() bool { return false }
+
 func v2auth(ctx context.Context, client *gophercloud.ProviderClient, endpoint string, options tokens2.AuthOptionsBuilder, eo gophercloud.EndpointOpts) error {
 	v2Client, err := NewIdentityV2(client, eo)
 	if err != nil {
@@ -143,21 +149,8 @@ func v2auth(ctx context.Context, client *gophercloud.ProviderClient, endpoint st
 		tac.SetThrowaway(true)
 		tac.ReauthFunc = nil
 		tac.SetTokenAndAuthResult(nil)
-		var tao tokens2.AuthOptionsBuilder
-		switch ot := options.(type) {
-		case *gophercloud.AuthOptions:
-			o := *ot
-			o.AllowReauth = false
-			tao = &o
-		case *tokens2.AuthOptions:
-			o := *ot
-			o.AllowReauth = false
-			tao = &o
-		default:
-			tao = options
-		}
 		client.ReauthFunc = func(ctx context.Context) error {
-			err := v2auth(ctx, &tac, endpoint, tao, eo)
+			err := v2auth(ctx, &tac, endpoint, &v2TokenNoReauth{options}, eo)
 			if err != nil {
 				return err
 			}
diff --git a/openstack/client_test.go b/openstack/client_test.go
new file mode 100644
index 00000000..68a9fdb0
--- /dev/null
+++ b/openstack/client_test.go
@@ -0,0 +1,5 @@
+package openstack
+
+import tokens2 "github.com/gophercloud/gophercloud/v2/openstack/identity/v2/tokens"
+
+var _ tokens2.AuthOptionsBuilder = &v2TokenNoReauth{}