[ISSUE #12017] Split console authentication#12474
Merged
KomachiSion merged 4 commits intoalibaba:summer-ospp#12017from Aug 27, 2024
Merged
[ISSUE #12017] Split console authentication#12474KomachiSion merged 4 commits intoalibaba:summer-ospp#12017from
KomachiSion merged 4 commits intoalibaba:summer-ospp#12017from
Conversation
* Git Test
KomachiSion
reviewed
Aug 12, 2024
Collaborator
KomachiSion
left a comment
KomachiSion
left a comment
There was a problem hiding this comment.
It seems no relative with auth？
* Fix Git Test
* Add ApiType annotations * Update configuration file to add fields * Update console authentication status fetch * Update `application.properties` - Reuse `nacos.core.auth.enabled` and add `nacos.core.auth.console.enabled` * Update `AuthConfigs` - Modify `isAuthEnabled()` to manage plugin initialization * Update `AbstractProtocolAuthService` - Implement `isAuthEnabled()` method for `Secured` annotation and configuration-based authentication * Update `RemoteRequestAuthFilter` - Use `authConfigs.isAuthEnabled()` for initialization and `protocolAuthService.authEnabled(secured)` for authentication checks
Contributor
Author
This one is for testing commits, I'll fix this change |
KomachiSion
requested changes
Aug 15, 2024
| } | ||
|
|
||
| @Override | ||
| public boolean authEnabled(Secured secured) { |
Collaborator
There was a problem hiding this comment.
和上述的enableAuth区别是什么？
是不是应该放到NacosAuthPluginService的enableAuth里？
| * @param secured secured information | ||
| * @return {@code true} if auth is open, otherwise {@code false} | ||
| */ | ||
| boolean authEnabled(Secured secured); |
Collaborator
There was a problem hiding this comment.
和上一个enable auth看起来重复，让插件开发和摸不着头脑
| /** | ||
| * console API. | ||
| */ | ||
| ADMIN_API("ADMIN_API"), |
Collaborator
There was a problem hiding this comment.
我想了一下， 这里可能改叫CONSOLE_API比较好
Collaborator
There was a problem hiding this comment.
因为之后可能会单独出maintainer sdk访问admin api，同时拆分部署后console访问server的api可能才叫ADMIN API
所以我想目前的叫CONSOLE_API
当然之后统一改也可以
| } | ||
|
|
||
| Secured secured = method.getAnnotation(Secured.class); | ||
| if (!protocolAuthService.authEnabled(secured)) { |
* Update the location of the authentication judgment
KomachiSion
approved these changes
Aug 27, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please do not create a Pull Request without creating an issue first.
What is the purpose of the change
For #12017
Split console authentication.
Brief changelog
Add ApiType annotations
Update configuration file to add fields
Update console authentication status fetch
Update
application.properties- Reusenacos.core.auth.enabledand addnacos.core.auth.console.enabledUpdate
AuthConfigs- ModifyisAuthEnabled()to manage plugin initializationUpdate
AbstractProtocolAuthService- ImplementisAuthEnabled()method forSecuredannotation and configuration-based authenticationUpdate
RemoteRequestAuthFilter- UseauthConfigs.isAuthEnabled()for initialization andprotocolAuthService.authEnabled(secured)for authentication checksVerifying this change
XXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
[ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=trueto make sure basic checks pass. Runmvn clean install -DskipITsto make sure unit-test pass. Runmvn clean test-compile failsafe:integration-testto make sure integration-test pass.