-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature][Login] Add OAuth login #8706
Conversation
Hi @William-GuoWei @dailidong @break60 , I am Yang Luo from Casdoor team. Casdoor is a powerful open-source single-sign-on (SSO) platform. It has the following features:
There have been a lot of user requests about the SSO scenario. This integration proposal will greatly enhance the SSO feature of DolphinScheduler with better authentication and security protection. What do you think? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some comments about the naming. Maybe use Casdoor
might be better rather than sso
.
...scheduler-api/src/main/java/org/apache/dolphinscheduler/api/security/AuthenticationType.java
Outdated
Show resolved
Hide resolved
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/security/SecurityConfig.java
Outdated
Show resolved
Hide resolved
...pi/src/main/java/org/apache/dolphinscheduler/api/security/impl/sso/CasdoorAuthenticator.java
Show resolved
Hide resolved
@CalvinKirs @ruanwenjun I have fixed the CI error. Please approve to rerun :) Thanks❤️ |
Codecov Report
@@ Coverage Diff @@
## dev #8706 +/- ##
============================================
+ Coverage 38.20% 38.22% +0.01%
- Complexity 4436 4448 +12
============================================
Files 1220 1222 +2
Lines 42698 42733 +35
Branches 4734 4736 +2
============================================
+ Hits 16314 16333 +19
- Misses 24581 24593 +12
- Partials 1803 1807 +4
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Kudos, SonarCloud Quality Gate passed! |
@Abingcbc Thanks, right now we put the front-end code in dolphinscheduler-ui-next, and we will not maintain the dolphinscheduler-ui, could you please put your change in dolphinscheduler-ui-next. |
@ruanwenjun I have implemented the front-end in dolphinscheduler-ui-next :) PTAL |
SonarCloud Quality Gate failed. |
Hi @ruanwenjun any update on this PR? |
Hi @Abingcbc it's a good feature, can you resolve the conflicts? |
@caishunfeng I have solved the conflicts and updated the code. Furthermore, Casdoor is developing rapidly. There are a lot of new features since the last commit. So I add some codes to use a random OAuth state to prevent a CRSF attack. |
Sure, I will review it. |
@Abingcbc License check failed, could u plz fix it? |
import static org.apache.dolphinscheduler.api.enums.Status.IP_IS_EMPTY; | ||
import static org.apache.dolphinscheduler.api.enums.Status.SIGN_OUT_ERROR; | ||
import static org.apache.dolphinscheduler.api.enums.Status.USER_LOGIN_FAILURE; | ||
import static org.apache.dolphinscheduler.api.enums.Status.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid wildcard imports, CI blocks this line.
@Abingcbc Thanks for submitting this PR. This is an important feature, as we are also working on supporting other OAuth2 providers such as Google and GitHub #12549. I will help review this PR too. |
@hsluoyz Thanks for providing the detailed information and letting us know this fantastic tool. We will keep following up with the review and make sure it could get merged ASAP : ) |
@caishunfeng plz approve |
@hsluoyz Done, have approved and restarted the CI : ) |
# Authentication types (supported types: PASSWORD,LDAP) | ||
type: PASSWORD | ||
# Authentication types (supported types: PASSWORD,LDAP,CASDOOR_SSO) | ||
type: CASDOOR_SSO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can modify this in local tests, but have to revert to PASSWORD
when the PR is finalized before merging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will modify it back. THX!
The current version is just a draft for @EricGao888 to review.
@caishunfeng @EricGao888 any comments? |
@caishunfeng @EricGao888 @kezhenxu94 The migration to Spring Security brings significant changes to the existing login process. Besides SSO and OAuth (there are still problems with front-end redirects), the unified configuration and implementation of PASSWORD and LDAP also need to be considered. |
CI approved. |
Kudos, SonarCloud Quality Gate passed! |
I rerun the CI. |
Great , need this feature! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Abingcbc LGTM, however, u still need to resolve the conflicts and add related docs in both Chinese and English.
Kudos, SonarCloud Quality Gate passed! |
@caishunfeng PTAL thx :) |
vue: 3.2.39 | ||
vue-i18n: 9.2.2_vue@3.2.39 | ||
vue-router: 4.1.5_vue@3.2.39 | ||
'@antv/layout': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this PR update many pnpm lock version, @devosend can you check whether it have to change or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lock files can be changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for frontend
Purpose of the pull request
Add SSO login for Dolphinscheduler
This pull request will close
#4531
#3451
Usage
application.yaml
You can find client id and client secret in Casdoor's
application
page and jwt public key in Casdoor'scert
page.3. Add your redirect URL
http://localhost:8888/view/login/index.html
in Casdoor's application setting.4. Now start your DolphinScheduler and you can login by SSO in the way like the following video.
Brief change log
Old UI
Login.-.DolphinScheduler.-.Google.Chrome.2022-03-05.14-40-12.mp4
New UI
Screen.Recording.2022-04-10.at.22.55.34.mov
Verify this pull request
This change added tests and can be verified as follows: