-
Notifications
You must be signed in to change notification settings - Fork 8k
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
Add simple login page auth for sentinel-dashboard #659
Conversation
Codecov Report
@@ Coverage Diff @@
## master #659 +/- ##
============================================
+ Coverage 40.41% 40.76% +0.35%
- Complexity 1187 1214 +27
============================================
Files 267 267
Lines 7852 7911 +59
Branches 1062 1072 +10
============================================
+ Hits 3173 3225 +52
- Misses 4281 4291 +10
+ Partials 398 395 -3
Continue to review full report at Codecov.
|
Wow, cool! I'll take review these days. |
@sczyh30 Thanks for inspiration! I tried to make the login page beautyer, but found it a bit troublesome to adjust the Bootstrap style. Maybe the background style like the home page of official website is good looking, I need your help. |
sentinel-dashboard/src/main/java/com/alibaba/csp/sentinel/dashboard/config/WebConfig.java
Outdated
Show resolved
Hide resolved
...el-dashboard/src/main/java/com/alibaba/csp/sentinel/dashboard/controller/AuthController.java
Outdated
Show resolved
Hide resolved
sentinel-dashboard/src/main/webapp/resources/app/scripts/controllers/login.js
Outdated
Show resolved
Hide resolved
Yes, background style like the home page of the official website is nice, but it could be difficult to implement. I'll try to improve this later :) |
Change sessionStorage to localStorage, to keep session in different tab page in browser. If user has logined, jump to the index page directly. Use English error msg instead of Chinese. Change session key from sentinel_admin to session_sentinel_admin.
Oops... I found a bug, when user logined but then the server restarted, the main page will be displayed, but the app can't be obtained. I need some time to fix it. |
…e status code to 401 Add authInterceptor in app.js, when 401 return clear session in localStorage and jump to the login page Change localStorage.clear() to localStorage.removeItem Format js which modifyed with 2 space indent, to keep same style with other js files
@sczyh30 I find a way to fix the bug, please take a review. To verify: |
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.
Can we support session expiration? For example, the session will be expired automatically after a period of inactivity.
SpringBoot support web server session expiration, we can add a property in application.properties e.g. also can be configured by VM argument: The default value is 30min. public class Session {
/**
* Session timeout. If a duration suffix is not specified, seconds will be used.
*/
@DurationUnit(ChronoUnit.SECONDS)
private Duration timeout = Duration.ofMinutes(30);
... Note: private long getSessionTimeoutInMinutes() {
Duration sessionTimeout = getSession().getTimeout();
if (isZeroOrLess(sessionTimeout)) {
return 0;
}
return Math.max(sessionTimeout.toMinutes(), 1);
} The default 30min maybe ok for most common scene. If the web session expired, the localStorage will be cleared by |
Good, we can add the description in README.md. |
...el-dashboard/src/main/java/com/alibaba/csp/sentinel/dashboard/controller/AuthController.java
Outdated
Show resolved
Hide resolved
If authUsername or authPassword is set to blank, auth will pass.
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
Awesome! Thanks for contributing! |
Thanks for your contribution, perfect! |
Describe what this PR does / why we need it
Sentinel dashboard provide interfaces for authentication/authorization since 1.5.0, and users can customize it to meet their requirement such as access to their own account/login system.
But some users may not want to modify the code, and just want a simple auth mechanism(login by username and password), like dubbo-admin does.
Does this pull request fix one issue?
Related to #517 and #143
Describe how you did it
Add a login page which accept the input of username and password, and login.js, auth_service.js.
Add
AuthController
class which provide login and logout method, the login method auth the input from login page.The default username and password is sentinel/sentinel, which can also be configured by JVM argument 、System env or property,
e.g:
-Dauth.username=sentinel
-Dauth.password=123456
If auth passed, put the AuthUser data in HttpSession.
Add
SimpleWebAuthServiceImpl
class which implements AuthService, get the AuthUser data from HttpSession.Modify
WebConfig
class, app.js, header.js to auth both front and back endsDescribe how to verify it
Start dashboard, visit
http://localhost:8080/
http://localhost:8080/index_dev.htm#/dashboard/app/sentinel-dashboard
http://localhost:8080/app/sentinel-dashboard/machines.json
The login page will be redirected.
Input sentinel/sentinel, then dashboard function pages can be used.
Click the logout button in the top right will back to the login page.
Special notes for reviews
Now the login page is very simple, which can be beauty, and maybe we can add some common useful infos on it, such as FAQ link, important issues and so on.