apache / apache/cloudstack

OAuth2 Google login permanently breaks under concurrent logins due to shared mutable token state in GoogleOAuth2Provider

Đang mở
#13,563 1 bình luận 0 reaction 0 người được giao Được @rhenar0 nhận Xem trên GitHub
bug
Ngôn ngữ chính
Java
Star
3.1k
Fork
1.4k
Merge trung bình
6 ngày 19 giờ
Pull request đã merge (30 ngày)
32

Mô tả

### problem

When several users log in via Google OAuth2 at the same time (e.g. a classroom of students connecting at the start of a lab session), some logins fail with `Unable to verify the email address with the provided secret`, and then all subsequent Google OAuth2 logins fail until `cloudstack-management` is restarted. From the end user's perspective, the OAuth2 SSO looks like it "disabled itself".

`GoogleOAuth2Provider` is a singleton Spring bean, but it caches the OAuth `accessToken` / `refreshToken` in shared mutable instance fields, making the login flow non-thread-safe.
[https://github.com/apache/cloudstack/blob/main/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java](https://github.com/apache/cloudstack/blob/main/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java)

```
public class GoogleOAuth2Provider extends AdapterBase implements UserOAuth2Authenticator {

protected String accessToken = null;
protected String refreshToken = null;
...

@Override
public String verifyCodeAndFetchEmail(String secretCode) {
...
if (StringUtils.isAnyEmpty(accessToken, refreshToken)) {
GoogleTokenResponse tokenResponse = flow.newTokenRequest(secretCode)
.setRedirectUri(redirectURI)
.execute();
accessToken = tokenResponse.getAccessToken();
refreshToken = tokenResponse.getRefreshToken();
}

GoogleCredential credential = ... .setAccessToken(accessToken).setRefreshToken(refreshToken);
Userinfo userinfo = oauth2.userinfo().get().execute();
return userinfo.getEmail();
}
```
And in `verifyUser()`:

```
String verifiedEmail = verifyCodeAndFetchEmail(secretCode);
if (verifiedEmail == null || !email.equals(verifiedEmail)) {
throw new CloudRuntimeException("Unable to verify the email address with the provided secret");
}
clearAccessAndRefreshTokens(); // <-- only reached on the success path
```

Two compounding problems:

1. Race condition between concurrent logins.
The tokens are shared by all in-flight logins. If user A's tokens are stored and user B enters `verifyCodeAndFetchEmail()` before A reaches `clearAccessAndRefreshTokens()`, the `isAnyEmpty(...)` guard is false, so B skips exchanging its own authorization code and calls userinfo with A's tokens. Google returns A's email, which does not match B's, so `verifyUser()` throws.

2. Poisoned state is never cleaned up.
`clearAccessAndRefreshTokens()` is only called on the success path of `verifyUser()`. Any failure (the email mismatch above, or an `IOException` on the `userinfo` call) leaves the stale tokens in the singleton. From then on, every subsequent login skips its own code exchange and reuses the stale/foreign token:
- while the stolen access token is still valid, `userinfoù returns the original user's email → email mismatch → all other users fail;
- once it expires (and the refresh token is invalid/not usable), `userinfo` fails with 401 → everyone fails.

The only way to reset the singleton's fields is to restart the management server, which matches the observed behavior exactly.

Note that the token caching provides no benefit at all: each login carries its own one-time authorization code that must be exchanged individually. The instance fields are simply a bug.

### versions

CloudStack 4.22.0
Management server: Rocky Linux, single management server
Hypervisors: KVM
Configuration: oauth2.enabled = true, Google registered as OAuth2 provider (client ID / secret / redirect URI), used as the primary login method for a school environment (~hundreds of users)

### The steps to reproduce the bug

1. Enable OAuth2 (oauth2.enabled = true) and register Google as a provider.
2. Have several distinct users (distinct Google accounts, each mapped to a CloudStack user) initiate the Google OAuth2 login flow concurrently (a handful of simultaneous logins is enough; the more concurrency, the easier it triggers.)
3. Observe some logins failing with Unable to verify the email address with the provided secret.
4. After the first such failure, try any new Google OAuth2 login (any user, any browser): it fails too. All Google OAuth2 logins keep failing until cloudstack-management is restarted.

The race can also be demonstrated deterministically in a debugger by pausing one thread between the token assignment and `clearAccessAndRefreshTokens()` while a second thread runs `verifyCodeAndFetchEmail()`.

### What to do about it?

Remove the shared instance fields and use local variables. Every call must exchange its own authorization code:

```
@Override
public String verifyCodeAndFetchEmail(String secretCode) {
...
GoogleTokenResponse tokenResponse;
try {
tokenResponse = flow.newTokenRequest(secretCode)
.setRedirectUri(redirectURI)
.execute();
} catch (IOException e) {
throw new CloudRuntimeException(String.format("Failed to exchange the authorization code: %s", e.getMessage()));
}

String accessToken = tokenResponse.getAccessToken(); // local
String refreshToken = tokenResponse.getRefreshToken(); // local

GoogleCredential credential = new GoogleCredential.Builder()
.setTransport(httpTransport)
.setJsonFactory(jsonFactory)
.setClientSecrets(clientSecrets)
.build()
.setAccessToken(accessToken)
.setRefreshToken(refreshToken);
...
}
```

And delete the accessToken/refreshToken fields together with `clearAccessAndRefreshTokens()` (also removing its call site in verifyUser()).

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

Bắt đầu trong plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java, đặc biệt là verifyCodeAndFetchEmail() và verifyUser(). Theo dõi cách các mã ủy quyền trở thành thông tin xác thực trong các lần gọi đồng thời và cách các lỗi được xử lý. Hoàn thành có nghĩa là mỗi lần đăng nhập sử dụng trạng thái token riêng, các lần thử thất bại không ảnh hưởng đến những lần đăng nhập sau đó và hành vi OAuth2 liên quan được xác minh.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
java
Lĩnh vực
authentication, backend
Loại issue
Lỗi
Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức độ hoạt động
Đình trệ
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
35/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.