Add support for restricted data stores#30
Merged
orpiske merged 1 commit intowanaku-ai:mainfrom Nov 20, 2025
Merged
Conversation
Reviewer's GuideThis PR implements support for restricted data stores by extracting shared OAuth2 authentication components into a new security module and refactoring existing discovery and services clients to use generic ServiceAuthenticator and ServiceConfig interfaces. Class diagram for refactored authentication and configuration classesclassDiagram
class ServiceAuthenticator {
- SecurityServiceConfig config
- AccessToken accessToken
- RefreshToken refreshToken
- Instant creationTime
+ ServiceAuthenticator(SecurityServiceConfig config)
- renewToken(SecurityServiceConfig config)
- createTokenRequest(SecurityServiceConfig config)
- getClientAuthentication(SecurityServiceConfig config)
- requestToken(TokenRequest request)
+ toHeaderValue()
}
class SecurityServiceConfig {
<<interface>>
+ getClientId() String
+ getSecret() String
+ getTokenEndpoint() String
}
class ServiceConfig {
<<interface>>
+ getBaseUrl() String
+ getSerializer() Serializer
+ getClientId() String
+ getSecret() String
+ getTokenEndpoint() String
}
class DefaultServiceConfig {
+ baseUrl String
+ serializer Serializer
+ clientId String
+ secret String
+ tokenEndpoint String
+ DefaultServiceConfig(Builder builder)
+ getBaseUrl() String
+ getSerializer() Serializer
+ getClientId() String
+ getSecret() String
+ getTokenEndpoint() String
+ Builder
}
class ServiceAuthException {
+ ServiceAuthException()
+ ServiceAuthException(String message)
+ ServiceAuthException(String message, Throwable cause)
+ ServiceAuthException(Throwable cause)
+ ServiceAuthException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace)
}
ServiceAuthenticator --> SecurityServiceConfig
ServiceConfig --|> SecurityServiceConfig
DefaultServiceConfig --|> ServiceConfig
ServiceAuthenticator ..> ServiceAuthException
Class diagram for updated HTTP client classesclassDiagram
class DiscoveryServiceHttpClient {
- String serviceBasePath
- ServiceAuthenticator serviceAuthenticator
- String baseUrl
- Serializer serializer
- HttpClient httpClient
+ DiscoveryServiceHttpClient(ServiceConfig config)
- sanitize(ServiceConfig config) String
+ executePost(...)
+ ping(String id)
+ updateState(String id, ServiceState serviceState)
}
class ServicesHttpClient {
- String baseUrl
- Serializer serializer
- ObjectMapper objectMapper
- ServiceAuthenticator serviceAuthenticator
- HttpClient httpClient
+ ServicesHttpClient(ServiceConfig config)
- sanitize(ServiceConfig config) String
+ executePost(...)
+ executePut(...)
+ executeGet(...)
+ executeDelete(...)
}
DiscoveryServiceHttpClient --> ServiceAuthenticator
ServicesHttpClient --> ServiceAuthenticator
DiscoveryServiceHttpClient --> ServiceConfig
ServicesHttpClient --> ServiceConfig
Class diagram for removed and replaced classesclassDiagram
class DiscoveryAuthenticator {
- DiscoveryServiceConfig config
- AccessToken accessToken
- RefreshToken refreshToken
- Instant creationTime
+ DiscoveryAuthenticator(DiscoveryServiceConfig config)
- renewToken(DiscoveryServiceConfig config)
- createTokenRequest(DiscoveryServiceConfig config)
- getClientAuthentication(DiscoveryServiceConfig config)
- requestToken(TokenRequest request)
}
class DiscoveryServiceConfig {
<<interface>>
+ getBaseUrl() String
+ getSerializer() Serializer
+ getClientId() String
+ getSecret() String
+ getTokenEndpoint() String
}
class DefaultDiscoveryServiceConfig {
+ baseUrl String
+ serializer Serializer
+ clientId String
+ secret String
+ tokenEndpoint String
+ DefaultDiscoveryServiceConfig(Builder builder)
+ getBaseUrl() String
+ getSerializer() Serializer
+ getClientId() String
+ getSecret() String
+ getTokenEndpoint() String
+ Builder
}
class ServicesClientConfig {
<<interface>>
+ getBaseUrl() String
+ getSerializer() Serializer
+ getClientId() String
+ getSecret() String
+ getTokenEndpoint() String
}
class DefaultServicesClientConfig {
+ baseUrl String
+ serializer Serializer
+ clientId String
+ secret String
+ tokenEndpoint String
+ DefaultServicesClientConfig(Builder builder)
+ getBaseUrl() String
+ getSerializer() Serializer
+ getClientId() String
+ getSecret() String
+ getTokenEndpoint() String
+ Builder
}
class DiscoveryAuthException {
+ DiscoveryAuthException()
+ DiscoveryAuthException(String message)
+ DiscoveryAuthException(String message, Throwable cause)
+ DiscoveryAuthException(Throwable cause)
+ DiscoveryAuthException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace)
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- There’s duplicated sanitize(baseUrl) logic in both HTTP clients—consider extracting it into a shared helper to reduce code repetition.
- Since you’ve renamed DiscoveryServiceConfig/DiscoveryAuthenticator to ServiceConfig/ServiceAuthenticator, add deprecation aliases or a brief migration note so existing clients won’t break immediately.
- You’re building HTTP requests with auth headers in multiple places—think about centralizing request construction or auth injection to make future extensions easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s duplicated sanitize(baseUrl) logic in both HTTP clients—consider extracting it into a shared helper to reduce code repetition.
- Since you’ve renamed DiscoveryServiceConfig/DiscoveryAuthenticator to ServiceConfig/ServiceAuthenticator, add deprecation aliases or a brief migration note so existing clients won’t break immediately.
- You’re building HTTP requests with auth headers in multiple places—think about centralizing request construction or auth injection to make future extensions easier.
## Individual Comments
### Comment 1
<location> `capabilities-discovery/src/main/java/ai/wanaku/capabilities/sdk/discovery/DiscoveryAuthenticator.java:113` </location>
<code_context>
TokenErrorResponse errorResponse = response.toErrorResponse();
- LOG.error("Unable to authenticate with discovery service: {}", errorResponse.getErrorObject().getDescription());
+ LOG.error("Unable to authenticate with service: {}", errorResponse.getErrorObject().getDescription());
throw new RuntimeException(errorResponse.getErrorObject().getDescription());
}
</code_context>
<issue_to_address>
**suggestion:** Throwing RuntimeException on authentication error may obscure error handling.
Use a specific exception like ServiceAuthException for authentication failures to improve error handling consistency.
Suggested implementation:
```java
LOG.error("Unable to authenticate with service: {}", errorResponse.getErrorObject().getDescription());
throw new ServiceAuthException(errorResponse.getErrorObject().getDescription());
```
```java
import com.nimbusds.oauth2.sdk.AccessTokenResponse;
import com.nimbusds.oauth2.sdk.AuthorizationGrant;
import com.nimbusds.oauth2.sdk.ClientCredentialsGrant;
import org.slf4j.LoggerFactory;
import ai.wanaku.capabilities.sdk.discovery.ServiceAuthException;
```
If `ServiceAuthException` does not already exist, you need to create it. For example, add this file:
`capabilities-discovery/src/main/java/ai/wanaku/capabilities/sdk/discovery/ServiceAuthException.java`:
```java
package ai.wanaku.capabilities.sdk.discovery;
public class ServiceAuthException extends RuntimeException {
public ServiceAuthException(String message) {
super(message);
}
public ServiceAuthException(String message, Throwable cause) {
super(message, cause);
}
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
...ies-discovery/src/main/java/ai/wanaku/capabilities/sdk/discovery/DiscoveryAuthenticator.java
Outdated
Show resolved
Hide resolved
- Also move the reusable security bits to a new module Ref: wanaku-ai/wanaku#640
orpiske
force-pushed
the
ci-issue-640-add-support-for-services-auth
branch
from
776c99a to
d33a2a7
Compare
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
1 participant
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.
Ref: wanaku-ai/wanaku#640
Summary by Sourcery
Centralize OAuth2 authentication logic into a new security module and refactor discovery and services clients to use common security abstractions
New Features:
Enhancements:
Build: