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

feat(server):swagger support auth for standardAuth mode #2360

Merged
merged 2 commits into from
Nov 28, 2023

Conversation

SunnyBoy-WYH
Copy link
Contributor

@SunnyBoy-WYH SunnyBoy-WYH commented Nov 22, 2023

…x arthas odd test

Purpose of the PR

swagger support auth for standardAuth mode and try to fix arthas odd test

Main Changes

  1. add swagger auth annotation.
  2. set openapi.json to whitelist.
  3. try to simplify arthas test,try to fix the odd test failed case.

Verifying these changes

no auth model,we can try it out in swagger:
24fbd7022f9f6a3ff361442ab6477bb
auth model,we cant try it out in swagger:
1d4bec0ddee89e86e5eef603b021dd1
after set auth info,success:
9c4b6f83e3c7e7ef4b02cad243f50b3

559b6f513d241a487f3d903c682d283
  • Trivial rework / code cleanup without any test coverage. (No Need)
  • Already covered by existing tests, such as (please modify tests here).
  • Need tests and can be verified as follows:
    • xxx

Does this PR potentially affect the following parts?

  • Nope
  • Dependencies (add/update license info)
  • Modify configurations
  • The public API
  • Other affects (typed here)

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

Copy link

codecov bot commented Nov 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (25301f6) 64.32% compared to head (f431efc) 63.43%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2360      +/-   ##
============================================
- Coverage     64.32%   63.43%   -0.90%     
+ Complexity      981      826     -155     
============================================
  Files           507      507              
  Lines         42073    42073              
  Branches       5831     5831              
============================================
- Hits          27065    26687     -378     
- Misses        12317    12685     +368     
- Partials       2691     2701      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -72,7 +72,8 @@ public class AuthenticationFilter implements ContainerRequestFilter {

private static final List<String> WHITE_API_LIST = ImmutableList.of(
"auth/login",
"versions"
"versions",
"openapi.json"
Copy link
Member

Choose a reason for hiding this comment

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

we add swagger file to white_list by default to allow normal users to check the api & not locked by auth

@imbajin
Copy link
Member

imbajin commented Nov 24, 2023

One more thing, we need update apiversion for metrics & arthas API here: (use 0.70 for them maybe?)
image

Also search 0.69 in all files and also change them to 0.70.

Note we can trace API changes through the commits history: https://github.com/apache/incubator-hugegraph/commits/27c0d1026ac85336b13137458e38950f3b1a43bb/hugegraph-api/src/main/java/org/apache/hugegraph/api

@SunnyBoy-WYH
Copy link
Contributor Author

One more thing, we need update apiversion for metrics & arthas API here: (use 0.70 for them maybe?) image

Also search 0.69 in all files and also change them to 0.70

but seems we dont have related issue to direct it, can we dont use this format?
like [0.69] Issue-1748: Support Cypher query RESTful API

@imbajin
Copy link
Member

imbajin commented Nov 24, 2023

One more thing, we need update apiversion for metrics & arthas API here: (use 0.70 for them maybe?) image

Also search 0.69 in all files and also change them to 0.70

but seems we dont have related issue to direct it, can we dont use this format?
like [0.69] Issue-1748: Support Cypher query RESTful API

issue= pr (number)here,and normally we need a issue before submit a pr

but here we could just use pr number as the issue number

@javeme
Copy link
Contributor

javeme commented Nov 25, 2023

please also supplement apiversion for #2242

@SunnyBoy-WYH
Copy link
Contributor Author

[0.69] Issue-1748: Support Cypher query RESTful API

seems [0.70] PR-999:Support Cypher query RESTful API OR ISSUE-999:Support Cypher query RESTful API,which 999 means pr number. seems the first is better

@imbajin
Copy link
Member

imbajin commented Nov 25, 2023

[0.69] Issue-1748: Support Cypher query RESTful API

seems [0.70] PR-999:Support Cypher query RESTful API OR ISSUE-999:Support Cypher query RESTful API,which 999 means pr number. seems the first is better

both fine to me(#number is also acceptable but not sure for others opinion),u could try modify it

@simon824 simon824 merged commit 0c01475 into apache:master Nov 28, 2023
18 of 21 checks passed
VGalaxies pushed a commit to VGalaxies/incubator-hugegraph that referenced this pull request Jan 12, 2024
* feat(server):swagger support auth for standardAuth mode and try to fix arthas odd test

* chore(api): update api version & swagger token auth mode
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.

5 participants