Skip to content

Commit

Permalink
chore: Override OAuth2AuthenticationException to differentiate the er…
Browse files Browse the repository at this point in the history
…rors thrown by Appsmith (appsmithorg#35160)

## Description
> Extend OAuth2AuthenticationException so that we can differentiate
between AppsmithException and exceptions thrown by Spring Library.
> There is not going to be any change to the Authentication flows here,
as the we are just inheriting the OAuth2AuthenticationException.


Fixes #`Issue Number`  
_or_  
Fixes `Issue URL`
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags="@tag.All"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10092949232>
> Commit: bc2f204
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10092949232&attempt=2"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.All`
> Spec:
> <hr>Thu, 25 Jul 2024 13:13:00 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit


- **New Features**
- Introduced a new custom exception for improved handling of OAuth 2.0
authentication errors, enhancing the clarity and robustness of the
authentication process.
  
- **Bug Fixes**
- Enhanced error categorization in the authentication process by
refining the error handling logic, allowing for better management of
exceptions related to OAuth 2.0.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Nilesh Sarupriya <[email protected]>
  • Loading branch information
2 people authored and MajaharZemoso committed Jul 28, 2024
1 parent 4661fc3 commit 67582c4
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@
import com.appsmith.server.domains.LoginSource;
import com.appsmith.server.domains.User;
import com.appsmith.server.domains.UserState;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.exceptions.AppsmithOAuth2AuthenticationException;
import com.appsmith.server.repositories.UserRepository;
import com.appsmith.server.services.UserService;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.security.oauth2.client.userinfo.DefaultReactiveOAuth2UserService;
import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest;
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
import org.springframework.security.oauth2.core.OAuth2Error;
import org.springframework.security.oauth2.core.user.OAuth2User;
import reactor.core.publisher.Mono;

Expand Down Expand Up @@ -65,6 +68,12 @@ private Mono<User> checkAndCreateUser(OAuth2User oAuth2User, OAuth2UserRequest u
return repository.save(user);
}
return Mono.just(user);
});
})
.onErrorMap(
AppsmithException.class,
// Throwing an AppsmithOAuth2AuthenticationException in case of an AppsmithException
// This is to differentiate between Appsmith exceptions and OAuth2 exceptions
error -> new AppsmithOAuth2AuthenticationException(
new OAuth2Error(error.getAppErrorCode().toString(), error.getMessage(), "")));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.appsmith.server.domains.User;
import com.appsmith.server.domains.UserState;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.exceptions.AppsmithOAuth2AuthenticationException;
import com.appsmith.server.repositories.UserRepository;
import com.appsmith.server.services.UserService;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -76,7 +77,9 @@ public Mono<User> checkAndCreateUser(OidcUser oidcUser, OidcUserRequest userRequ
})
.onErrorMap(
AppsmithException.class,
error -> new OAuth2AuthenticationException(
// Throwing an AppsmithOAuth2AuthenticationException in case of an AppsmithException
// This is to differentiate between Appsmith exceptions and OAuth2 exceptions
error -> new AppsmithOAuth2AuthenticationException(
new OAuth2Error(error.getAppErrorCode().toString(), error.getMessage(), "")));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.appsmith.server.exceptions;

import lombok.Getter;
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
import org.springframework.security.oauth2.core.OAuth2Error;

@Getter
public class AppsmithOAuth2AuthenticationException extends OAuth2AuthenticationException {

private final OAuth2Error error;
/**
* Constructs an {@code AppsmithOAuth2AuthenticationException} using the provided parameters.
* @param error the {@link OAuth2Error OAuth 2.0 Error}
*/
public AppsmithOAuth2AuthenticationException(OAuth2Error error) {
this(error, error.getDescription(), null);
}

/**
* Constructs an {@code AppsmithOAuth2AuthenticationException} using the provided parameters.
* @param error the {@link OAuth2Error OAuth 2.0 Error}
* @param message the detail message
* @param cause the root cause
*/
public AppsmithOAuth2AuthenticationException(OAuth2Error error, String message, Throwable cause) {
super(error, message, cause);
this.error = error;
}
}

0 comments on commit 67582c4

Please sign in to comment.