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 LoginAPI #1500

Merged
merged 15 commits into from
Jun 24, 2021
Merged

add LoginAPI #1500

merged 15 commits into from
Jun 24, 2021

Conversation

corgiboygsj
Copy link
Member

Use HS256 generator token, token payload have user_name and user_id.

hugegraph-core/pom.xml Show resolved Hide resolved

@Watched(prefix = "cache")
@Override
public boolean update(K id, V value, long timeOffset) {
Copy link
Contributor

Choose a reason for hiding this comment

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

rebase master

@codecov
Copy link

codecov bot commented Jun 17, 2021

Codecov Report

Merging #1500 (a94a071) into master (0935c75) will decrease coverage by 1.55%.
The diff coverage is 80.44%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1500      +/-   ##
============================================
- Coverage     60.78%   59.22%   -1.56%     
- Complexity      675     6124    +5449     
============================================
  Files           423      410      -13     
  Lines         35051    33213    -1838     
  Branches       4967     4587     -380     
============================================
- Hits          21305    19670    -1635     
+ Misses        11601    11478     -123     
+ Partials       2145     2065      -80     
Impacted Files Coverage Δ
.../com/baidu/hugegraph/auth/ConfigAuthenticator.java 0.00% <0.00%> (ø)
...a/com/baidu/hugegraph/auth/HugeGraphAuthProxy.java 57.35% <25.00%> (+1.40%) ⬆️
...ain/java/com/baidu/hugegraph/api/auth/UserAPI.java 75.80% <33.33%> (-2.16%) ⬇️
...c/main/java/com/baidu/hugegraph/auth/HugeUser.java 88.07% <66.66%> (-0.61%) ⬇️
...in/java/com/baidu/hugegraph/api/auth/LoginAPI.java 74.28% <74.28%> (ø)
...va/com/baidu/hugegraph/auth/HugeAuthenticator.java 39.56% <75.00%> (+5.85%) ⬆️
...idu/hugegraph/api/filter/AuthenticationFilter.java 54.45% <75.75%> (+10.01%) ⬆️
.../java/com/baidu/hugegraph/auth/TokenGenerator.java 88.88% <88.88%> (ø)
...n/java/com/baidu/hugegraph/config/AuthOptions.java 92.85% <92.85%> (ø)
...om/baidu/hugegraph/auth/StandardAuthenticator.java 41.42% <100.00%> (+6.04%) ⬆️
... and 82 more

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 0935c75...a94a071. Read the comment docs.

@javeme
Copy link
Contributor

javeme commented Jun 18, 2021

to add api test

@corgiboygsj corgiboygsj force-pushed the auth_token branch 2 times, most recently from a7cbbb3 to f40e567 Compare June 21, 2021 09:02
@javeme
Copy link
Contributor

javeme commented Jun 21, 2021

also bump api version

javeme
javeme previously approved these changes Jun 21, 2021
public RolePermission loginUser(String username, String password);
public String loginUser(String username, String password)
throws AuthenticationException;
public void logoutUser(String token);
Copy link
Member

@imbajin imbajin Jun 22, 2021

Choose a reason for hiding this comment

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

maybe better to return boolean, same as the logout-api / client (or unify with DELETE api together to make user friendly)

public StandardAuthManager(HugeGraphParams graph) {
E.checkNotNull(graph, "graph");

this.graph = graph;
this.eventListener = this.listenChanges();
this.usersCache = this.cache("users");
this.pwdCache = this.cache("users_pwd");
this.tokenCache = this.cache("token");
Copy link
Member

Choose a reason for hiding this comment

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

should the token expired for 1day by default? (or set to 1h?)

Copy link
Member Author

Choose a reason for hiding this comment

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

1h is too short, no token refresh in current version

javeme
javeme previously approved these changes Jun 22, 2021
imbajin
imbajin previously approved these changes Jun 22, 2021
@corgiboygsj corgiboygsj dismissed stale reviews from imbajin and javeme via a1fe010 June 22, 2021 09:31
@javeme javeme merged commit 9ee9fe1 into master Jun 24, 2021
@javeme javeme deleted the auth_token branch June 24, 2021 14:50
jadepeng pushed a commit to jadepeng/hugegraph that referenced this pull request Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants