Item14248: Corrected configuration keys

foswiki · Dec 9, 2016 · fae3d52 · fae3d52
1 parent 9a0f925
commit fae3d52
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 16 deletions.
diff --git a/lib/Foswiki/Contrib/OpenIDLoginContrib/Config.spec b/lib/Foswiki/Contrib/OpenIDLoginContrib/Config.spec
@@ -3,7 +3,7 @@
 # ---+++ Provider Details
 # **URL LABEL="OpenID Connect Configuration URL"**
 # An URL that points to the OpenID Connect discovery document.
-# It usually ends in .well-known/openid-configuration.
+# It should end in /.well-known/openid-configuration.
 $Foswiki::cfg{Extensions}{OpenID}{Default}{DiscoveryURL} = '';
 # **URL LABEL="Redirect/OAuth Callback URL"**
 # The callback URL that the OpenID provider redirects to. This
@@ -16,17 +16,18 @@ $Foswiki::cfg{Extensions}{OpenID}{Default}{ClientID} = '';
 # The client secret should be provided to you by your Open ID Provider
 $Foswiki::cfg{Extensions}{OpenID}{Default}{ClientSecret} = '';
 # **REGEX LABEL="Issuer Regex Match" CHECK="undefok emptyok"**
-# OpenID works with id tokens issued by an identity provider. This REGEX lets
+# OpenID works with ID tokens issued by an identity provider. This regex lets
 # you specify which identity providers you trust. If you leave this empty, the
 # issuer value from the discovery document will be used. However, some providers,
-# such as Microsoft Azure AD, issue id tokens with issuer values that depend on the
-# tenant. In that case, you must provide a regex which matches the issuer value.
-# Look at the 'iss' key in the discovery document of your Open ID Provider for the
-# exact format.
+# such as Microsoft Azure AD, host multiple tenants, all of them will have their
+# own issuer identities. The discovery document can't list all of them, so in
+# such a case you must provide a regex to manually match the correct issuer string.
+# Look at the 'issuer' key in the discovery document of your Open ID Provider for 
+# the format to expect.
 $Foswiki::cfg{Extensions}{OpenID}{Default}{IssuerRegex} = '';
 # ---+++ Users
 # **STRING LABEL="WikiName Attributes"**
-# Comma-separated ID Token attributes which should make up the wiki name. The default
+# Comma-separated ID token attributes which should make up the wiki name. The default
 # should give good results.
 $Foswiki::cfg{Extensions}{OpenID}{Default}{WikiNameAttributes} = 'given_name,family_name';
 # **STRING LABEL="Loginname Attribute"**

diff --git a/lib/Foswiki/Contrib/OpenIDLoginContrib/OpenIDConnect.pm b/lib/Foswiki/Contrib/OpenIDLoginContrib/OpenIDConnect.pm
@@ -12,7 +12,7 @@ sub endpoint_discovery {
     my $discovery_uri = shift;
     my $ua = LWP::UserAgent->new;
     my $response = $ua->get($discovery_uri);
-    die "Could not retrieve Open ID endpoint configuration" if !$response->is_success;
+    die "Could not retrieve Open ID endpoint configuration at $discovery_uri" if !$response->is_success;
     return JSON::decode_json($response->decoded_content);
 }
 
@@ -98,7 +98,7 @@ sub verify_id_token {
 		die "JWT ID token verification failed: " . $@;
 	    };
 	    die "JWT ID token verification failed: wrong audience" unless $audience eq $data->{'aud'};
-	    die "JWT ID token verification failed: wrong issuer" unless $issuer eq $data->{'iss'};
+	    die "JWT ID token verification failed: wrong issuer (" . $data->{'iss'} . ")" unless $data->{'iss'} =~ /$issuer/;
 	    return $data;
 	}
     }

diff --git a/lib/Foswiki/LoginManager/OpenIDConnectLogin.pm b/lib/Foswiki/LoginManager/OpenIDConnectLogin.pm
@@ -63,12 +63,12 @@ sub loadProviderData {
     my $provider = shift;
     # TODO: We should cache this. On sites with heavy traffic, this adds needless delays, especially since
     # we need to load it twice for each login
-    my $discovery_uri = $Foswiki::cfg{'Extensions'}{'OpenID'}{$provider}{'DiscoveryURI'};
+    my $discovery_uri = $Foswiki::cfg{'Extensions'}{'OpenID'}{$provider}{'DiscoveryURL'};
     $this->{endpoints} = endpoint_discovery($discovery_uri);
     $this->{client_id} = $Foswiki::cfg{'Extensions'}{'OpenID'}{$provider}{'ClientID'};
     $this->{client_secret} = $Foswiki::cfg{'Extensions'}{'OpenID'}{$provider}{'ClientSecret'};
-    $this->{issuer} = $Foswiki::cfg{'Extensions'}{'OpenID'}{$provider}{'IssuerRegex'};
-    $this->{redirect_uri} = $Foswiki::cfg{'Extensions'}{'OpenID'}{$provider}{'RedirectURI'};
+    $this->{issuer} = $Foswiki::cfg{'Extensions'}{'OpenID'}{$provider}{'IssuerRegex'} || $this->{endpoints}->{'issuer'};
+    $this->{redirect_uri} = $Foswiki::cfg{'Extensions'}{'OpenID'}{$provider}{'RedirectURL'};
     $this->{loginname_attr} = $Foswiki::cfg{'Extensions'}{'OpenID'}{$provider}{'LoginnameAttribute'};
     $this->{wikiname_attrs} = $Foswiki::cfg{'Extensions'}{'OpenID'}{$provider}{'WikiNameAttributes'};   
 }
@@ -194,7 +194,7 @@ sub redirectToProvider {
     my $origin = $query->param('foswiki_origin');
     # Avoid accidental passthrough
     $query->delete( 'foswiki_origin', 'provider' );
-
+    
     $this->loadProviderData($provider);
 
     my $request_uri = $this->build_auth_request($session, $origin);
@@ -342,7 +342,7 @@ sub login {
     my $state = $query->param('state');
     my $code = $query->param('code');
     my $password = $query->param('password');
-
+    
     # The login method now acts as a switchboard. When the provider
     # parameter is provided, we do an oauth redirect to the given
     # provider. When we get state and code parameters, we're running
@@ -352,14 +352,14 @@ sub login {
     # - it provides a way to access the original behaviour of the
     # TemplateLogin.
 
-    if ($provider && ($provider ne 'native')) {
+    if ((defined $provider) && ($provider ne 'native')) {
 	$this->redirectToProvider($provider, $query, $session);
 	return;
     }
     elsif ($state && $code) {
 	$this->oauthCallback($code, $state, $query, $session);
     }
-    elsif ($password || $provider eq 'native') {
+    elsif ($password || ((defined $provider) && ($provider eq 'native'))) {
 	# if we get a password or a request for the native login 
 	# provider, we redirect to the original TemplateLogin
 	$this->SUPER::login($query, $session);