Skip to content

Commit

Permalink
OIDC: update facebook now that it returns id tokens
Browse files Browse the repository at this point in the history
Also clean up Apple properties that we don't actually support ATM
  • Loading branch information
FroMage committed Jan 21, 2022
1 parent 4da4bad commit dedfad2
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,8 @@ public static class Authentication {
* Force 'https' as the 'redirect_uri' parameter scheme when running behind an SSL terminating reverse proxy.
* This property, if enabled, will also affect the logout `post_logout_redirect_uri` and the local redirect requests.
*/
@ConfigItem(defaultValue = "false")
public boolean forceRedirectHttpsScheme;
@ConfigItem(defaultValueDocumentation = "false")
public Optional<Boolean> forceRedirectHttpsScheme = Optional.empty();

/**
* List of scopes
Expand Down Expand Up @@ -603,12 +603,12 @@ public void setExtraParams(Map<String, String> extraParams) {
this.extraParams = extraParams;
}

public boolean isForceRedirectHttpsScheme() {
public Optional<Boolean> isForceRedirectHttpsScheme() {
return forceRedirectHttpsScheme;
}

public void setForceRedirectHttpsScheme(boolean forceRedirectHttpsScheme) {
this.forceRedirectHttpsScheme = forceRedirectHttpsScheme;
this.forceRedirectHttpsScheme = Optional.of(forceRedirectHttpsScheme);
}

public boolean isRestorePathAfterRedirect() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ private static void addExtraParamsToUri(StringBuilder builder, Map<String, Strin
}

private boolean isForceHttps(TenantConfigContext configContext) {
return configContext.oidcConfig.authentication.forceRedirectHttpsScheme;
return configContext.oidcConfig.authentication.forceRedirectHttpsScheme.orElse(false);
}

private Uni<Void> buildLogoutRedirectUriUni(RoutingContext context, TenantConfigContext configContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,9 @@ static OidcTenantConfig mergeTenantConfig(OidcTenantConfig tenant, OidcTenantCon
if (tenant.authentication.scopes.isEmpty()) {
tenant.authentication.scopes = provider.authentication.scopes;
}
if (tenant.authentication.forceRedirectHttpsScheme.isEmpty()) {
tenant.authentication.forceRedirectHttpsScheme = provider.authentication.forceRedirectHttpsScheme;
}

// credentials
if (tenant.credentials.clientSecret.method.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.quarkus.oidc.runtime.providers;

import java.util.HashMap;
import java.util.List;

import io.quarkus.oidc.OidcTenantConfig;
Expand Down Expand Up @@ -63,10 +62,8 @@ private static OidcTenantConfig facebook() {
ret.setAuthorizationPath("https://facebook.com/dialog/oauth/");
ret.setTokenPath("https://graph.facebook.com/v12.0/oauth/access_token");
ret.setJwksPath("https://www.facebook.com/.well-known/oauth/openid/jwks/");
ret.setUserInfoPath("https://graph.facebook.com/me/?fields=id,name,email,first_name,last_name");
ret.getAuthentication().setScopes(List.of("email", "public_profile"));
ret.getAuthentication().setUserInfoRequired(true);
ret.getAuthentication().setIdTokenRequired(false);
ret.getAuthentication().setForceRedirectHttpsScheme(true);
return ret;
}

Expand All @@ -75,8 +72,6 @@ private static OidcTenantConfig apple() {
ret.setAuthServerUrl("https://appleid.apple.com/");
ret.setApplicationType(OidcTenantConfig.ApplicationType.WEB_APP);
ret.getAuthentication().setScopes(List.of("openid", "email", "name"));
ret.getAuthentication().setExtraParams(new HashMap<>());
ret.getAuthentication().getExtraParams().put("response_mode", "form_post");
ret.getAuthentication().setForceRedirectHttpsScheme(true);
ret.getCredentials().getClientSecret().setMethod(Method.POST_JWT);
ret.getCredentials().getJwt().setSignatureAlgorithm(SignatureAlgorithm.ES256.getAlgorithm());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,9 @@ public void testAcceptFacebookProperties() throws Exception {
assertEquals("https://facebook.com/dialog/oauth/", config.getAuthorizationPath().get());
assertEquals("https://www.facebook.com/.well-known/oauth/openid/jwks/", config.getJwksPath().get());
assertEquals("https://graph.facebook.com/v12.0/oauth/access_token", config.getTokenPath().get());
assertEquals("https://graph.facebook.com/me/?fields=id,name,email,first_name,last_name",
config.getUserInfoPath().get());

assertFalse(config.authentication.idTokenRequired.get());
assertTrue(config.authentication.userInfoRequired.get());
assertEquals(List.of("email", "public_profile"), config.authentication.scopes.get());
assertTrue(config.authentication.forceRedirectHttpsScheme.get());
}

@Test
Expand All @@ -113,11 +110,9 @@ public void testOverrideFacebookProperties() throws Exception {
tenant.setAuthorizationPath("authorization");
tenant.setJwksPath("jwks");
tenant.setTokenPath("tokens");
tenant.setUserInfoPath("userinfo");

tenant.authentication.setIdTokenRequired(true);
tenant.authentication.setUserInfoRequired(false);
tenant.authentication.setScopes(List.of("write"));
tenant.authentication.setForceRedirectHttpsScheme(false);

OidcTenantConfig config = OidcUtils.mergeTenantConfig(tenant, KnownOidcProviders.provider(Provider.FACEBOOK));

Expand All @@ -126,12 +121,10 @@ public void testOverrideFacebookProperties() throws Exception {
assertTrue(config.isDiscoveryEnabled().get());
assertEquals("http://localhost/wiremock", config.getAuthServerUrl().get());
assertEquals("authorization", config.getAuthorizationPath().get());
assertFalse(config.getAuthentication().isForceRedirectHttpsScheme().get());
assertEquals("jwks", config.getJwksPath().get());
assertEquals("tokens", config.getTokenPath().get());
assertEquals("userinfo", config.getUserInfoPath().get());

assertTrue(config.authentication.idTokenRequired.get());
assertFalse(config.authentication.userInfoRequired.get());
assertEquals(List.of("write"), config.authentication.scopes.get());
}

Expand Down Expand Up @@ -186,6 +179,7 @@ public void testOverrideMicrosoftProperties() throws Exception {
tenant.setAuthServerUrl("http://localhost/wiremock");
tenant.getToken().setIssuer("http://localhost/wiremock");
tenant.authentication.setScopes(List.of("write"));
tenant.authentication.setForceRedirectHttpsScheme(false);

OidcTenantConfig config = OidcUtils.mergeTenantConfig(tenant, KnownOidcProviders.provider(Provider.MICROSOFT));

Expand All @@ -194,6 +188,7 @@ public void testOverrideMicrosoftProperties() throws Exception {
assertEquals("http://localhost/wiremock", config.getAuthServerUrl().get());
assertEquals(List.of("write"), config.authentication.scopes.get());
assertEquals("http://localhost/wiremock", config.getToken().getIssuer().get());
assertFalse(config.authentication.forceRedirectHttpsScheme.get());
}

@Test
Expand All @@ -209,6 +204,7 @@ public void testAcceptAppleProperties() throws Exception {
assertEquals(Method.POST_JWT, config.credentials.clientSecret.method.get());
assertEquals("https://appleid.apple.com/", config.credentials.jwt.audience.get());
assertEquals(SignatureAlgorithm.ES256.getAlgorithm(), config.credentials.jwt.signatureAlgorithm.get());
assertTrue(config.authentication.forceRedirectHttpsScheme.get());
}

@Test
Expand Down

0 comments on commit dedfad2

Please sign in to comment.