Skip to content

Commit a62ab7c

Browse files
committed
Fix OAuth callback ignoring per-team overrides; make config multi-tenant
(#653)
1 parent eba408c commit a62ab7c

9 files changed

Lines changed: 348 additions & 63 deletions

File tree

accounts/src/main/java/org/restheart/accounts/oauth/OAuthCallback.java

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public void handle(StringRequest req, StringResponse res) throws Exception {
126126
// Extract provider from path
127127
var parts = req.getPath().split("/");
128128
if (parts.length < 5) {
129-
redirectError(res, "Invalid callback path");
129+
redirectError(res, req, "Invalid callback path");
130130
return;
131131
}
132132
var provider = parts[4].toLowerCase();
@@ -135,19 +135,19 @@ public void handle(StringRequest req, StringResponse res) throws Exception {
135135
var errorParam = req.getQueryParameters().get("error");
136136
if (errorParam != null && !errorParam.isEmpty()) {
137137
LOGGER.warn("OAuth provider returned error: {}", errorParam.getFirst());
138-
redirectError(res, "Provider error: " + errorParam.getFirst());
138+
redirectError(res, req, "Provider error: " + errorParam.getFirst());
139139
return;
140140
}
141141

142142
var codeParam = req.getQueryParameters().get("code");
143143
var stateParam = req.getQueryParameters().get("state");
144144

145145
if (codeParam == null || codeParam.isEmpty()) {
146-
redirectError(res, "Missing authorization code");
146+
redirectError(res, req, "Missing authorization code");
147147
return;
148148
}
149149
if (stateParam == null || stateParam.isEmpty()) {
150-
redirectError(res, "Missing state parameter");
150+
redirectError(res, req, "Missing state parameter");
151151
return;
152152
}
153153

@@ -156,7 +156,7 @@ public void handle(StringRequest req, StringResponse res) throws Exception {
156156

157157
try {
158158
// 1. Exchange code + verify state → user profile + invite context
159-
var callbackResult = oauthService.handleCallback(provider, code, state);
159+
var callbackResult = oauthService.handleCallback(provider, code, state, req);
160160
var profile = callbackResult.profile();
161161
var email = profile.getString("email").getValue();
162162

@@ -217,7 +217,7 @@ public void handle(StringRequest req, StringResponse res) throws Exception {
217217
return;
218218
}
219219
LOGGER.info("OAuth login denied for invited user <{}>: activateViaOAuth returned empty", email);
220-
redirectError(res, "Account is pending activation");
220+
redirectError(res, req, "Account is pending activation");
221221
return;
222222
}
223223

@@ -227,7 +227,7 @@ public void handle(StringRequest req, StringResponse res) throws Exception {
227227
var inviteOpt = db.findInvitationByEmailAndToken(email, pendingInviteToken);
228228
if (inviteOpt.isEmpty()) {
229229
LOGGER.warn("OAuth invite acceptance failed: no valid invitation for <{}> with the supplied token", email);
230-
redirectError(res, "Invalid or expired invitation token");
230+
redirectError(res, req, "Invalid or expired invitation token");
231231
return;
232232
}
233233
var invite = inviteOpt.get();
@@ -268,10 +268,10 @@ public void handle(StringRequest req, StringResponse res) throws Exception {
268268

269269
} catch (OAuthService.OAuthException e) {
270270
LOGGER.warn("OAuth callback error ({}): {}", provider, e.getMessage());
271-
redirectError(res, e.getMessage());
271+
redirectError(res, req, e.getMessage());
272272
} catch (Exception e) {
273273
LOGGER.error("Unexpected error in OAuth callback ({})", provider, e);
274-
redirectError(res, "Internal error");
274+
redirectError(res, req, "Internal error");
275275
}
276276
}
277277

@@ -296,7 +296,7 @@ private void setAuthCookieAndRedirect(StringResponse res, StringRequest req, Str
296296
// `flow=signup` doubles as the one-shot "welcome banner" marker also used by
297297
// EmailVerificationService, so both signup paths signal the frontend the same way.
298298
var query = flow != null ? "?flow=" + flow : "";
299-
var baseUrl = oauthConfig.frontendSuccessUrl() + query;
299+
var baseUrl = RequestOverrides.oauthFrontendSuccessUrl(req, oauthConfig) + query;
300300
var location = delivery == TokenDelivery.Mode.COOKIE
301301
? baseUrl
302302
: TokenDelivery.fragmentUrl(baseUrl, conf, jwtToken);
@@ -417,9 +417,10 @@ private static String extractLastName(String fullName) {
417417
return parts.length > 1 ? parts[1] : "";
418418
}
419419

420-
private void redirectError(StringResponse res, String reason) throws Exception {
421-
var sep = oauthConfig.frontendErrorUrl().contains("?") ? "&" : "?";
422-
var url = oauthConfig.frontendErrorUrl() + sep + "reason="
420+
private void redirectError(StringResponse res, StringRequest req, String reason) throws Exception {
421+
var frontendErrorUrl = RequestOverrides.oauthFrontendErrorUrl(req, oauthConfig);
422+
var sep = frontendErrorUrl.contains("?") ? "&" : "?";
423+
var url = frontendErrorUrl + sep + "reason="
423424
+ URLEncoder.encode(reason, StandardCharsets.UTF_8);
424425
res.setStatusCode(HttpStatus.SC_TEMPORARY_REDIRECT);
425426
res.getHeaders().put(Headers.LOCATION, url);

accounts/src/main/java/org/restheart/accounts/oauth/OAuthConfig.java

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,34 @@ public boolean isProviderEnabled(String name) {
146146
return p != null && p.isValid();
147147
}
148148

149-
/** Returns the absolute callback URL for the given provider name. */
149+
/**
150+
* Returns the absolute callback URL for the given provider name, using the static
151+
* {@code api-base-url}.
152+
*
153+
* @deprecated Use {@link #callbackUrl(String, String)} with the effective (possibly
154+
* per-team overridden) base URL from {@code RequestOverrides.oauthApiBaseUrl(req, this)} —
155+
* a single static value cannot be correct on a multi-tenant node.
156+
*/
157+
@Deprecated
150158
public String callbackUrl(String providerName) {
159+
return callbackUrl(providerName, apiBaseUrl);
160+
}
161+
162+
/** Returns the absolute callback URL for the given provider name and base URL. */
163+
public String callbackUrl(String providerName, String apiBaseUrl) {
151164
return apiBaseUrl + "/auth/oauth/callback/" + providerName.toLowerCase();
152165
}
153166

167+
/**
168+
* Well-known default OAuth scope for a built-in provider name (e.g. {@code "google"},
169+
* {@code "github"}), or {@code ""} if the name is not one of them. Used as the last-resort
170+
* scope fallback when neither a per-team override nor a static {@code providers.{name}}
171+
* YAML entry supplies one.
172+
*/
173+
public static String defaultScope(String providerName) {
174+
return DEFAULT_SCOPES.getOrDefault(providerName == null ? "" : providerName.toLowerCase(), "");
175+
}
176+
154177
// ── Nested record ─────────────────────────────────────────────────────────
155178

156179
/**
@@ -181,7 +204,7 @@ public boolean isValid() {
181204

182205
@SuppressWarnings("unchecked")
183206
private ProviderConfig parseProviderConfig(String name, Map<String, Object> map) {
184-
var defaultScope = DEFAULT_SCOPES.getOrDefault(name, "");
207+
var defaultScope = defaultScope(name);
185208
return new ProviderConfig(
186209
name,
187210
configVal(map, "enabled", false),

accounts/src/main/java/org/restheart/accounts/oauth/OAuthInitiator.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.restheart.accounts.oauth;
22

33
import io.undertow.util.Headers;
4+
import org.restheart.accounts.util.RequestOverrides;
45
import org.restheart.exchange.StringRequest;
56
import org.restheart.exchange.StringResponse;
67
import org.restheart.exchange.ExchangeKeys.METHOD;
@@ -77,7 +78,11 @@ public void handle(StringRequest req, StringResponse res) throws Exception {
7778
}
7879
var provider = parts[4].toLowerCase();
7980

80-
if (!oauthConfig.isProviderEnabled(provider)) {
81+
// Uses the request-aware check (not oauthConfig.isProviderEnabled(), which only sees
82+
// static config) so a provider configured exclusively via per-team overrides — no
83+
// providers.{provider} entry in YAML at all, by design on a multi-tenant node — is
84+
// still recognized as available. See OAuthService.isProviderAvailable.
85+
if (!oauthService.isProviderAvailable(provider, req)) {
8186
res.setInError(HttpStatus.SC_BAD_REQUEST, "Provider '" + provider + "' is not enabled");
8287
return;
8388
}
@@ -89,7 +94,9 @@ public void handle(StringRequest req, StringResponse res) throws Exception {
8994
res.getHeaders().put(Headers.LOCATION, result.url());
9095
} catch (OAuthService.OAuthException e) {
9196
LOGGER.warn("OAuth authorize error for {}: {}", provider, e.getMessage());
92-
var errorUrl = oauthConfig.frontendErrorUrl() + "&reason="
97+
var frontendErrorUrl = RequestOverrides.oauthFrontendErrorUrl(req, oauthConfig);
98+
var sep = frontendErrorUrl.contains("?") ? "&" : "?";
99+
var errorUrl = frontendErrorUrl + sep + "reason="
93100
+ URLEncoder.encode(e.getMessage(), StandardCharsets.UTF_8);
94101
res.setStatusCode(HttpStatus.SC_TEMPORARY_REDIRECT);
95102
res.getHeaders().put(Headers.LOCATION, errorUrl);

accounts/src/main/java/org/restheart/accounts/oauth/OAuthService.java

Lines changed: 46 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.bson.BsonString;
1010
import org.restheart.plugins.accounts.AccountsConfigData;
1111
import org.restheart.accounts.util.RequestOverrides;
12+
import org.restheart.exchange.ServiceRequest;
1213
import org.restheart.plugins.Inject;
1314
import org.restheart.plugins.OnInit;
1415
import org.restheart.plugins.PluginRecord;
@@ -89,7 +90,7 @@ public class OAuthService implements Provider<OAuthService>, OAuthProviderRegist
8990

9091
// ── Helpers ───────────────────────────────────────────────────────────────
9192

92-
private static String queryParam(org.restheart.exchange.ServiceRequest<?> req, String name) {
93+
private static String queryParam(ServiceRequest<?> req, String name) {
9394
if (req == null) return null;
9495
var values = req.getQueryParameters().get(name);
9596
return (values != null && !values.isEmpty()) ? values.getFirst() : null;
@@ -133,24 +134,24 @@ public void registerProvider(OAuthProvider provider) {
133134
* @return an {@link AuthResult} with the authorization redirect URL and the CSRF state
134135
* @throws OAuthException if the provider is not configured or not registered
135136
*/
136-
public AuthResult getAuthorizationUrl(String providerName,
137-
org.restheart.exchange.ServiceRequest<?> req) throws OAuthException {
137+
public AuthResult getAuthorizationUrl(String providerName, ServiceRequest<?> req) throws OAuthException {
138138
var cfg = resolveProviderConfig(providerName, req);
139-
var provider = resolveProvider(providerName);
140-
var teamDb = RequestOverrides.db(req, accountsConf);
141-
var state = generateState(teamDb);
142-
var pendingInviteToken = queryParam(req, "pendingInviteToken");
143-
var consentsAccepted = "true".equalsIgnoreCase(queryParam(req, "consentsAccepted"));
139+
var provider = resolveProvider(providerName);
140+
var teamDb = RequestOverrides.db(req, accountsConf);
141+
var apiBaseUrl = RequestOverrides.oauthApiBaseUrl(req, config);
142+
var state = generateState(teamDb);
143+
var pendingInviteToken = queryParam(req, "pendingInviteToken");
144+
var consentsAccepted = "true".equalsIgnoreCase(queryParam(req, "consentsAccepted"));
144145

145146
storeStateToken(state, providerName, teamDb, pendingInviteToken, consentsAccepted);
146147

147148
var url = provider.getAuthorizationUrl(cfg.clientId(), cfg.clientSecret(),
148-
config.callbackUrl(providerName), cfg.scope(), state);
149+
config.callbackUrl(providerName, apiBaseUrl), cfg.scope(), state);
149150
return new AuthResult(url, state);
150151
}
151152

152153
/**
153-
* @deprecated Use {@link #getAuthorizationUrl(String, org.restheart.exchange.ServiceRequest)} instead.
154+
* @deprecated Use {@link #getAuthorizationUrl(String, ServiceRequest)} instead.
154155
* @param providerName the OAuth provider name
155156
* @throws OAuthException if the provider is not configured or not registered
156157
*/
@@ -169,25 +170,32 @@ public AuthResult getAuthorizationUrl(String providerName) throws OAuthException
169170
* @param providerName the OAuth provider name extracted from the callback path
170171
* @param code the authorization code received from the OAuth provider
171172
* @param state the CSRF state token returned by the provider (must match a stored token)
173+
* @param req the incoming callback request, used to resolve per-team overrides;
174+
* may be {@code null} for non-HTTP use (falls back to static config)
172175
* @return a {@link CallbackResult} carrying the user profile and the invite
173176
* context that was stored in the state token at authorization time
174177
* @throws OAuthException if the state token is invalid/expired, the provider is
175178
* not registered, or the profile fetch fails
176179
*/
177-
public CallbackResult handleCallback(String providerName, String code, String state)
180+
public CallbackResult handleCallback(String providerName, String code, String state, ServiceRequest<?> req)
178181
throws OAuthException {
179182

180183
var token = verifyAndConsumeState(state, providerName);
181184
if (token == null) {
182185
throw new OAuthException("Invalid or expired state token (possible CSRF)");
183186
}
184187

185-
var cfg = resolveProviderConfig(providerName);
186-
var provider = resolveProvider(providerName);
188+
// Must resolve with the SAME overrides used to build the authorize URL — a
189+
// per-team override changes both clientId/clientSecret and, via apiBaseUrl below,
190+
// the callbackUrl passed to the provider; the provider validates that the callback
191+
// token exchange uses the exact same callbackUrl/credentials as the authorize step.
192+
var cfg = resolveProviderConfig(providerName, req);
193+
var provider = resolveProvider(providerName);
194+
var apiBaseUrl = RequestOverrides.oauthApiBaseUrl(req, config);
187195

188196
try {
189197
var profile = provider.fetchUserProfile(cfg.clientId(), cfg.clientSecret(),
190-
config.callbackUrl(providerName), cfg.scope(), code);
198+
config.callbackUrl(providerName, apiBaseUrl), cfg.scope(), code);
191199
return new CallbackResult(profile, token.pendingInviteToken(), token.consentsAccepted());
192200
} catch (OAuthException e) {
193201
throw e;
@@ -282,25 +290,42 @@ private OAuthConfig.ProviderConfig resolveProviderConfig(String name) throws OAu
282290
}
283291

284292
/**
285-
* Resolves provider config, checking per-team overrides before falling
286-
* back to static config (currently only Google supports overrides).
293+
* Resolves provider config, checking per-team overrides before falling back to
294+
* static config. Provider-agnostic — works for any provider name, not just Google.
287295
*
288-
* @param name provider name (case-insensitive), e.g. {@code "google"}
296+
* @param name provider name (case-insensitive), e.g. {@code "google"}, {@code "github"}
289297
* @param req the incoming request used to read per-team overrides; may be {@code null}
290298
* @return the effective {@link OAuthConfig.ProviderConfig}
291299
* @throws OAuthException if OAuth is disabled or the provider is not configured
292300
*/
293-
public OAuthConfig.ProviderConfig resolveProviderConfig(String name,
294-
org.restheart.exchange.ServiceRequest<?> req) throws OAuthException {
295-
if ("google".equalsIgnoreCase(name) && req != null) {
296-
var teamCfg = org.restheart.accounts.util.RequestOverrides.oauthGoogle(req);
301+
public OAuthConfig.ProviderConfig resolveProviderConfig(String name, ServiceRequest<?> req)
302+
throws OAuthException {
303+
if (req != null) {
304+
var teamCfg = RequestOverrides.oauthProvider(req, name, config);
297305
if (teamCfg != null && teamCfg.isValid()) {
298306
return teamCfg;
299307
}
300308
}
301309
return resolveProviderConfig(name);
302310
}
303311

312+
/**
313+
* Whether {@code name} is usable for this request — either via a per-team override
314+
* ({@link RequestOverrides#oauthProvider}) or the static config. Use this instead of
315+
* {@link OAuthConfig#isProviderEnabled(String)} whenever a request is available: the
316+
* static-only check rejects providers that are configured <em>exclusively</em> via
317+
* per-team overrides (e.g. a multi-tenant node with no {@code providers.{name}} YAML
318+
* entry at all, by design — see {@code RequestOverrides} class docs).
319+
*/
320+
public boolean isProviderAvailable(String name, ServiceRequest<?> req) {
321+
try {
322+
resolveProviderConfig(name, req);
323+
return true;
324+
} catch (OAuthException e) {
325+
return false;
326+
}
327+
}
328+
304329
private OAuthProvider resolveProvider(String name) throws OAuthException {
305330
var p = providers.get(name.toLowerCase());
306331
if (p == null) throw new OAuthException("Provider '" + name + "' is not registered");

0 commit comments

Comments
 (0)