Skip to content
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

Merged
merged 8 commits into from
Apr 20, 2019

Conversation

cdfive
Copy link
Collaborator

@cdfive cdfive commented Apr 7, 2019

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

  1. Add a login page which accept the input of username and password, and login.js, auth_service.js.

  2. 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.

  3. Add SimpleWebAuthServiceImpl class which implements AuthService, get the AuthUser data from HttpSession.

  4. Modify WebConfig class, app.js, header.js to auth both front and back ends

Describe 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.

@codecov-io
Copy link

codecov-io commented Apr 7, 2019

Codecov Report

Merging #659 into master will increase coverage by 0.35%.
The diff coverage is n/a.

Impacted file tree graph

@@             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
Impacted Files Coverage Δ Complexity Δ
...baba/csp/sentinel/adapter/reactor/EntryConfig.java 80% <0%> (-6.96%) 9% <0%> (ø)
.../java/com/alibaba/csp/sentinel/util/SpiLoader.java 0% <0%> (ø) 0% <0%> (ø) ⬇️
...alibaba/csp/sentinel/node/metric/MetricWriter.java 58.57% <0%> (+2.95%) 21% <0%> (+2%) ⬆️
.../slots/block/flow/controller/WarmUpController.java 82.69% <0%> (+3.84%) 11% <0%> (+1%) ⬆️
...java/com/alibaba/csp/sentinel/util/StringUtil.java 48.07% <0%> (+32.69%) 28% <0%> (+18%) ⬆️
...ain/java/com/alibaba/csp/sentinel/util/IdUtil.java 100% <0%> (+100%) 6% <0%> (+6%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 29f22e3...07cd357. Read the comment docs.

@sczyh30 sczyh30 added area/dashboard Issues or PRs about Sentinel Dashboard to-review To review labels Apr 8, 2019
@sczyh30
Copy link
Member

sczyh30 commented Apr 8, 2019

Wow, cool! I'll take review these days.

@cdfive
Copy link
Collaborator Author

cdfive commented Apr 8, 2019

@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.

@sczyh30
Copy link
Member

sczyh30 commented Apr 11, 2019

@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.

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.
@cdfive
Copy link
Collaborator Author

cdfive commented Apr 11, 2019

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
@cdfive
Copy link
Collaborator Author

cdfive commented Apr 12, 2019

@sczyh30 I find a way to fix the bug, please take a review.
Add authInterceptor in app.js, when auth failed, AuthFilter return response status code 401, and clear session in localStorage and jump to the login page.

To verify:
Start server=>login=>close server=>F5 or click another menu, then will jump to the login page.

@sczyh30 sczyh30 added this to the 1.6.0 milestone Apr 12, 2019
Copy link
Member

@sczyh30 sczyh30 left a 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.

@cdfive
Copy link
Collaborator Author

cdfive commented Apr 15, 2019

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.
server.servlet.session.timeout=600 // 600 second
server.servlet.session.timeout=20m // 20 minute

also can be configured by VM argument:
-Dserver.servlet.session.timeout=600
-Dserver.servlet.session.timeout=20m

The default value is 30min.
Session class in org.springframework.boot.web.servlet.server package.

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:
If set the value in second, it will be convert to mintue by dividing the number of seconds by 60.
getSessionTimeoutInMinutes method in TomcatServletWebServerFactory class:

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.
Do we need to add this VM configuration parameter to README.md?

If the web session expired, the localStorage will be cleared by AuthInterceptor in app.js.

@sczyh30
Copy link
Member

sczyh30 commented Apr 16, 2019

Good, we can add the description in README.md.

If authUsername or authPassword is set to blank, auth will pass.
Copy link
Member

@sczyh30 sczyh30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sczyh30 sczyh30 merged commit 4acb907 into alibaba:master Apr 20, 2019
@sczyh30
Copy link
Member

sczyh30 commented Apr 20, 2019

Awesome! Thanks for contributing!

@CarpenterLee
Copy link
Contributor

Thanks for your contribution, perfect!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dashboard Issues or PRs about Sentinel Dashboard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants