m2m-oauth-server: Managing M2M OAuth Server Tokens #1350
Conversation
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 85 files out of 192 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughThe recent updates to the M2M OAuth server significantly enhance token management functionalities, including token creation, retrieval, blacklisting, and deletion. New features such as OpenID configuration support, additional constants for token management, and comprehensive gRPC service integration have been introduced. These improvements bolster security, streamline operations, and provide thorough documentation for easier implementation and scaling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant gRPC Server
participant MongoDB
participant OAuthSigner
Client->>gRPC Server: CreateToken Request
gRPC Server->>OAuthSigner: Validate Token Request
OAuthSigner-->>gRPC Server: Validation Response
gRPC Server->>MongoDB: Store Token
MongoDB-->>gRPC Server: Storage Response
gRPC Server-->>Client: CreateToken Response
Client->>gRPC Server: GetTokens Request
gRPC Server->>MongoDB: Retrieve Tokens
MongoDB-->>gRPC Server: Tokens Data
gRPC Server-->>Client: GetTokens Response
Client->>gRPC Server: BlacklistTokens Request
gRPC Server->>MongoDB: Update Token Status
MongoDB-->>gRPC Server: Update Response
gRPC Server-->>Client: BlacklistTokens Response
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Outside diff range, codebase verification and nitpick comments (3)
m2m-oauth-server/service/service_test.go (1)
1-22: Consider removing the print statement.Printing the configuration might not be necessary in the test function.
- fmt.Printf("cfg: %v\n", cfg.String())m2m-oauth-server/pb/token.go (1)
13-29: Enhance error messages for validation.Consider adding more specific error messages to help identify which field is invalid.
- if x.GetId() == "" { - return errors.New("Token.Id is empty") - } - if x.GetOwner() == "" { - return errors.New("Token.Owner is empty") - } + if x.GetId() == "" { + return fmt.Errorf("Token.Id is empty: %v", x) + } + if x.GetOwner() == "" { + return fmt.Errorf("Token.Owner is empty: %v", x) + }m2m-oauth-server/pb/README.md (1)
6-15: Fix indentation in the Table of Contents.The unordered list items should have consistent indentation.
- - [BlacklistTokensRequest](#m2moauthserver-pb-BlacklistTokensRequest) - - [BlacklistTokensResponse](#m2moauthserver-pb-BlacklistTokensResponse) - - [CreateTokenRequest](#m2moauthserver-pb-CreateTokenRequest) - - [CreateTokenResponse](#m2moauthserver-pb-CreateTokenResponse) - - [GetTokensRequest](#m2moauthserver-pb-GetTokensRequest) - - [Token](#m2moauthserver-pb-Token) - - [Token.BlackListed](#m2moauthserver-pb-Token-BlackListed) - - [M2MOAuthService](#m2moauthserver-pb-M2MOAuthService) + - [BlacklistTokensRequest](#m2moauthserver-pb-BlacklistTokensRequest) + - [BlacklistTokensResponse](#m2moauthserver-pb-BlacklistTokensResponse) + - [CreateTokenRequest](#m2moauthserver-pb-CreateTokenRequest) + - [CreateTokenResponse](#m2moauthserver-pb-CreateTokenResponse) + - [GetTokensRequest](#m2moauthserver-pb-GetTokensRequest) + - [Token](#m2moauthserver-pb-Token) + - [Token.BlackListed](#m2moauthserver-pb-Token-BlackListed) + - [M2MOAuthService](#m2moauthserver-pb-M2MOAuthService)Tools
LanguageTool
[duplication] ~12-~12: Possible typo: you repeated a word
Context: ...oauthserver-pb-GetTokensRequest) - Token - [Token.BlackListed](#m2moauthserver-pb-Token-B...(ENGLISH_WORD_REPEAT_RULE)
Markdownlint
7-7: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
8-8: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
9-9: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
10-10: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
11-11: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
13-13: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
15-15: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (6)
m2m-oauth-server/config.yamlis excluded by!**/*.yamlm2m-oauth-server/pb/service.pb.gois excluded by!**/*.pb.go,!**/*.pb.gom2m-oauth-server/pb/service.pb.gw.gois excluded by!**/*.pb.gw.go,!**/*.pb.gw.gom2m-oauth-server/pb/service.swagger.jsonis excluded by!**/*.jsonm2m-oauth-server/pb/service_grpc.pb.gois excluded by!**/*.pb.go,!**/*.pb.gom2m-oauth-server/swagger.yamlis excluded by!**/*.yaml
Files selected for processing (48)
- Makefile (1 hunks)
- m2m-oauth-server/Makefile (2 hunks)
- m2m-oauth-server/oauthSigner/config.go (1 hunks)
- m2m-oauth-server/oauthSigner/loadKeys.go (1 hunks)
- m2m-oauth-server/oauthSigner/oauthSigner.go (1 hunks)
- m2m-oauth-server/pb/README.md (1 hunks)
- m2m-oauth-server/pb/doc.html (1 hunks)
- m2m-oauth-server/pb/service.proto (1 hunks)
- m2m-oauth-server/pb/tags.go (1 hunks)
- m2m-oauth-server/pb/token.go (1 hunks)
- m2m-oauth-server/service/config.go (2 hunks)
- m2m-oauth-server/service/expiredUpdatesChecker.go (1 hunks)
- m2m-oauth-server/service/grpc/config.go (1 hunks)
- m2m-oauth-server/service/grpc/server.go (1 hunks)
- m2m-oauth-server/service/grpc/service.go (1 hunks)
- m2m-oauth-server/service/grpc/token.go (1 hunks)
- m2m-oauth-server/service/http/config.go (1 hunks)
- m2m-oauth-server/service/http/createToken_test.go (1 hunks)
- m2m-oauth-server/service/http/getJWKs.go (2 hunks)
- m2m-oauth-server/service/http/getJWKs_test.go (1 hunks)
- m2m-oauth-server/service/http/getOpenIDConfiguration.go (2 hunks)
- m2m-oauth-server/service/http/getOpenIDConfiguration_test.go (1 hunks)
- m2m-oauth-server/service/http/getTokens_test.go (1 hunks)
- m2m-oauth-server/service/http/jsonWriter.go (1 hunks)
- m2m-oauth-server/service/http/postToken.go (1 hunks)
- m2m-oauth-server/service/http/postToken_test.go (5 hunks)
- m2m-oauth-server/service/http/requestHandler.go (1 hunks)
- m2m-oauth-server/service/http/service.go (1 hunks)
- m2m-oauth-server/service/service.go (1 hunks)
- m2m-oauth-server/service/service_test.go (1 hunks)
- m2m-oauth-server/store/config/config.go (1 hunks)
- m2m-oauth-server/store/config/config_test.go (1 hunks)
- m2m-oauth-server/store/cqldb/config.go (1 hunks)
- m2m-oauth-server/store/cqldb/store.go (1 hunks)
- m2m-oauth-server/store/cqldb/tokens.go (1 hunks)
- m2m-oauth-server/store/mongodb/config.go (1 hunks)
- m2m-oauth-server/store/mongodb/store.go (1 hunks)
- m2m-oauth-server/store/mongodb/tokens.go (1 hunks)
- m2m-oauth-server/store/mongodb/tokens_test.go (1 hunks)
- m2m-oauth-server/store/store.go (1 hunks)
- m2m-oauth-server/test/service.go (1 hunks)
- m2m-oauth-server/test/test.go (4 hunks)
- m2m-oauth-server/uri/uri.go (1 hunks)
- pkg/security/jwt/claims.go (1 hunks)
- pkg/security/jwt/validator/validator.go (3 hunks)
- pkg/security/openid/config.go (1 hunks)
- snippet-service/service/http/service.go (2 hunks)
- test/config/config.go (1 hunks)
Files skipped from review due to trivial changes (2)
- m2m-oauth-server/pb/tags.go
- m2m-oauth-server/service/grpc/config.go
Additional context used
LanguageTool
m2m-oauth-server/pb/README.md
[duplication] ~12-~12: Possible typo: you repeated a word
Context: ...oauthserver-pb-GetTokensRequest) - Token - [Token.BlackListed](#m2moauthserver-pb-Token-B...(ENGLISH_WORD_REPEAT_RULE)
Markdownlint
m2m-oauth-server/pb/README.md
7-7: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
8-8: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
9-9: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
10-10: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
11-11: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
13-13: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
15-15: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
137-137: null
Link fragments should be valid(MD051, link-fragments)
126-126: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style(MD055, table-pipe-style)
Additional comments not posted (153)
m2m-oauth-server/store/cqldb/store.go (2)
7-9: LGTM!The struct definition is simple and correctly embeds another struct.
3-5: Ensure necessary imports.The import statement looks fine, but ensure that
github.com/plgd-dev/hub/v2/pkg/cqldbis necessary and used correctly.Verification successful
Necessary import confirmed.
The import statement for
github.com/plgd-dev/hub/v2/pkg/cqldbinm2m-oauth-server/store/cqldb/store.gois necessary and used correctly within the codebase.
m2m-oauth-server/store/cqldb/store.go: Thecqldbpackage is used in this file.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `github.com/plgd-dev/hub/v2/pkg/cqldb` in the codebase. # Test: Search for the usage of `cqldb` package. Expect: Relevant usages in the codebase. rg --type go 'cqldb'Length of output: 18122
m2m-oauth-server/store/mongodb/config.go (3)
7-9: LGTM!The struct definition is simple and correctly includes the MongoDB configuration.
11-13: LGTM!The
Validatemethod correctly delegates validation to the embedded MongoDB configuration.
3-5: Ensure necessary imports.The import statement looks fine, but ensure that
github.com/plgd-dev/hub/v2/pkg/mongodbis necessary and used correctly.Verification successful
The import is necessary and used correctly.
The import
github.com/plgd-dev/hub/v2/pkg/mongodbis used to define theConfigstruct and its validation method inm2m-oauth-server/store/mongodb/config.go.
m2m-oauth-server/store/mongodb/config.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `github.com/plgd-dev/hub/v2/pkg/mongodb` in the codebase. # Test: Search for the usage of `mongodb` package. Expect: Relevant usages in the codebase. rg --type go 'mongodb'Length of output: 56230
Script:
#!/bin/bash # Description: Extract the content of `m2m-oauth-server/store/mongodb/config.go` to verify the usage of `pkgMongo`. # Extract the content of the file cat m2m-oauth-server/store/mongodb/config.goLength of output: 255
m2m-oauth-server/store/cqldb/config.go (3)
7-11: LGTM!The struct definition is simple and correctly includes the CQDB configuration.
13-15: LGTM!The
Validatemethod correctly delegates validation to the embedded CQDB configuration.
3-5: Ensure necessary imports.The import statement looks fine, but ensure that
github.com/plgd-dev/hub/v2/pkg/cqldbis necessary and used correctly.Verification successful
Ensure necessary imports.
The import statement for
github.com/plgd-dev/hub/v2/pkg/cqldbinm2m-oauth-server/store/cqldb/config.gois necessary and used correctly.
m2m-oauth-server/store/cqldb/config.go: Thecqldbpackage is used within this file.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `github.com/plgd-dev/hub/v2/pkg/cqldb` in the codebase. # Test: Search for the usage of `cqldb` package. Expect: Relevant usages in the codebase. rg --type go 'cqldb'Length of output: 18122
m2m-oauth-server/service/http/jsonWriter.go (2)
1-1: Package renaming LGTM!The package renaming from
servicetohttpis appropriate and reflects the reorganization of the codebase.
Line range hint
5-15:
LGTM!The
jsonResponseWriterfunction is well-implemented, handling both nil and non-nil responses correctly.m2m-oauth-server/service/http/getJWKs.go (1)
1-1: Package name change approved.The package name change from
servicetohttpis appropriate.m2m-oauth-server/service/http/config.go (1)
1-13: LGTM!The
Configstruct is well-defined and appropriate for HTTP service configuration.m2m-oauth-server/store/cqldb/tokens.go (4)
10-12: LGTM!The
CreateTokenfunction correctly returnsstore.ErrNotSupported.
14-16: LGTM!The
GetTokensfunction correctly returnsstore.ErrNotSupported.
18-20: LGTM!The
DeleteTokensfunction correctly returnsstore.ErrNotSupported.
22-24: LGTM!The
BlacklistTokensfunction correctly returnsstore.ErrNotSupported.m2m-oauth-server/oauthSigner/loadKeys.go (1)
Line range hint
1-21:
LGTM!The
LoadPrivateKeyfunction correctly handles different types of private keys and errors.However, ensure that all function calls to
LoadPrivateKeymatch the new package name.m2m-oauth-server/service/http/getOpenIDConfiguration.go (2)
11-18: LGTM!The
GetOpenIDConfigurationfunction correctly generates the OpenID configuration based on the provided domain.
20-24: LGTM!The
getOpenIDConfigurationmethod inRequestHandlercorrectly handles the response and error logging.pkg/security/openid/config.go (1)
13-13: LGTM!The
Configstruct correctly includes the new fieldPlgdTokensEndpointand maintains consistency.m2m-oauth-server/uri/uri.go (7)
8-8: LGTM!The constant
TokenNameKeyis correctly defined and follows the naming conventions.
11-11: LGTM!The constant
TimeToLiveKeyis correctly defined and follows the naming conventions.
17-17: LGTM!The constant
TokenTypeKeyis correctly defined and follows the naming conventions.
18-18: LGTM!The constant
ExpiresInKeyis correctly defined and follows the naming conventions.
23-23: LGTM!The constant
APIis correctly defined and follows the naming conventions.
27-27: LGTM!The constant
Tokensis correctly defined and follows the naming conventions.
28-28: LGTM!The constant
BlacklistTokensis correctly defined and follows the naming conventions.m2m-oauth-server/service/http/getJWKs_test.go (2)
1-1: LGTM!The package name change from
service_testtohttp_testis logical and consistent with the file's location and purpose.
17-18: LGTM!The addition of the
context.Contextparameter to the functiongetJWKsis a good practice for managing timeouts and cancellations. The implementation updates are correctly applied.Also applies to: 22-22, 25-25
m2m-oauth-server/store/config/config_test.go (3)
17-20: LGTM!The test case
validis correctly defined and covers a valid configuration scenario.
22-28: LGTM!The test case
valid - no cronis correctly defined and covers a valid configuration scenario with no cron expression.
30-37: LGTM!The test case
invalid - bad cron expressionis correctly defined and covers an invalid configuration scenario with a bad cron expression.m2m-oauth-server/service/grpc/service.go (2)
14-16: LGTM!The
Servicestruct is correctly defined and follows the embedding pattern.
18-34: LGTM!The
Newfunction is correctly defined, handles errors appropriately, and ensures graceful stopping of the server.m2m-oauth-server/service/http/getOpenIDConfiguration_test.go (2)
16-23: LGTM!The function correctly sets up the context and service, and defers teardown operations.
25-39: LGTM! But verify the endpoint and response structure.The function correctly sends a GET request to retrieve the OpenID configuration and validates the response.
However, ensure that the endpoint and the response structure are correct.
m2m-oauth-server/store/config/config.go (1)
20-43: LGTM! But verify proper error handling and logging.The function correctly validates the configuration and sets up the cron job for cleaning up deleted tokens.
However, ensure proper error handling and logging.
Verification successful
Verification Successful: Proper Error Handling and Logging
The
Validatefunction inm2m-oauth-server/store/config/config.gocorrectly implements error handling and logging.
- Error handling is present for scheduler creation and job addition.
- Logging is implemented for scheduler shutdown errors.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify proper error handling and logging in the Validate function. # Test: Search for error handling and logging in the Validate function. Expect: Proper error handling and logging. rg --type go --pattern 'func (c *Config) Validate() error'Length of output: 472
Script:
#!/bin/bash # Description: Verify proper error handling and logging in the Validate function. # Test: Search for error handling and logging in the Validate function. Expect: Proper error handling and logging. rg 'func (c \*Config) Validate\(\) error' -A 20Length of output: 49
Script:
#!/bin/bash # Description: Verify proper error handling and logging in the Validate function. # Test: Use ast-grep to search for the Validate function and verify proper error handling and logging. ast-grep --lang go --pattern $'func (c *Config) Validate() error { $$$ }'Length of output: 39874
snippet-service/service/http/service.go (1)
27-27: LGTM!The function correctly creates the HTTP service with the updated authorization rules.
m2m-oauth-server/service/http/requestHandler.go (2)
25-45: LGTM!The function correctly sets up the request handler and registers the routes.
17-22: LGTM!The struct is correctly defined with the necessary fields.
m2m-oauth-server/store/mongodb/store.go (4)
1-31: LGTM!The file-level and type-level declarations are appropriate.
33-49: LGTM!The function
Newis well-structured and handles errors appropriately.
52-60: LGTM!The function
clearDatabasesis clear and usesmultierrorto aggregate errors.
62-64: LGTM!The function
Closeis straightforward and appropriate.m2m-oauth-server/store/store.go (5)
1-28: LGTM!The file-level and type-level declarations are appropriate.
30-32: LGTM!The function
IsDuplicateKeyErroris straightforward and appropriate.
34-36: LGTM!The interface
BsonMapperis clear and appropriate.
38-61: LGTM!The type
MongoIteratorand its methods handle decoding and error checking appropriately.
63-75: LGTM!The interface
Storeis clear and defines necessary methods for token operations.m2m-oauth-server/test/service.go (2)
45-65: LGTM!The function
NewMongoStoreis clear and handles errors appropriately.
68-70: LGTM!The function
HTTPURIis straightforward and appropriate.m2m-oauth-server/service/http/service.go (2)
1-22: LGTM!The file-level and type-level declarations are appropriate.
24-68: LGTM!The function
Newis well-structured and handles errors appropriately.m2m-oauth-server/service/config.go (5)
5-9: LGTM! Imports are necessary and correctly used.The new imports for
oauthsigner,grpcService, andstoreConfigare required for the added functionalities.
17-27: LGTM! TheStoragefield is correctly integrated and validated.The
ClientsConfigstruct now includes aStoragefield, and its validation logic ensures proper configuration.
33-37: LGTM! TheOAuthSignerfield type change is correctly integrated and validated.The
Configstruct now includes theOAuthSignerfield of typeoauthsigner.Config, and its validation logic ensures proper configuration.Also applies to: 39-49
61-71: LGTM! TheGRPCfield is correctly integrated and validated.The
APIsConfigstruct now includes aGRPCfield, and its validation logic ensures proper configuration.
76-82: LGTM! TheAddrfield is correctly integrated and validated.The
HTTPConfigstruct now includes anAddrfield, and its validation logic ensures proper configuration. The removal of theConnectionfield is handled appropriately.m2m-oauth-server/service/http/createToken_test.go (2)
3-20: LGTM! Imports are necessary and correctly used.The new imports for various packages are required for the testing functionalities.
22-81: LGTM! TheTestCreateTokenfunction is comprehensive and correctly validates the token creation process.The test function sets up the necessary context, defines test cases, and ensures the token creation process is validated correctly.
pkg/security/jwt/validator/validator.go (3)
43-47: LGTM! The new types for handling OpenID configuration are correctly defined.The new types
GetOpenIDConfigurationFuncandOptionsare correctly defined and used for handling OpenID configuration.
49-53: LGTM! The new function for setting theGetOpenIDConfigurationoption is correctly defined.The new function
WithGetOpenIDConfigurationis correctly defined and used for setting theGetOpenIDConfigurationoption.
Line range hint
55-78: LGTM! The function is correctly modified to handle the new options and validate the OpenID configuration handling logic.The
Newfunction is correctly modified to accept additional options for OpenID configuration retrieval and includes proper validation logic.m2m-oauth-server/Makefile (2)
9-11: LGTM! The new variables are correctly defined and used.The new variables
GOPATH,WORKING_DIRECTORY, andREPOSITORY_DIRECTORYare correctly defined and used.
47-63: LGTM! The extendedproto/generatetarget is correctly defined and used.The
proto/generatetarget now includes additionalprotoccommands for generating various files related to protocol buffers and API documentation.m2m-oauth-server/oauthSigner/oauthSigner.go (7)
19-21: LGTM!The function correctly wraps and formats an error message.
31-62: LGTM!The constructor function correctly initializes the
OAuthSignerstruct, handles errors properly, and ensures resources are cleaned up if an error occurs.
64-67: LGTM!The method correctly retrieves the validator from the map based on the client ID.
69-83: LGTM!The method correctly sets the headers, signs the data using JWS, and handles errors properly.
85-87: LGTM!The method correctly returns the JWK key.
89-95: LGTM!The method correctly encodes the JWT token, signs it using
SignRaw, and handles errors properly.
97-99: LGTM!The method correctly ensures that resources are cleaned up by executing the closer function list.
m2m-oauth-server/service/http/postToken_test.go (1)
Line range hint
1-118:
LGTM!The test case correctly sets up the OAuth server, runs sub-tests for different scenarios, and validates the results using assertions.
m2m-oauth-server/oauthSigner/config.go (6)
12-14: LGTM!The type definition correctly defines a new string type for
AccessTokenType.
16-20: LGTM!The type definition correctly defines a new string type for
GrantTypewith a constant value.
22-35: LGTM!The struct definition correctly defines the fields for
PrivateKeyJWTConfig, and theValidatemethod ensures the configuration is valid.
37-83: LGTM!The struct definition correctly defines the fields for
Client, and theValidatemethod ensures the client configuration is valid.
85-94: LGTM!The type definition correctly defines a slice of
Clientpointers, and theFindmethod correctly retrieves a client by ID.
96-128: LGTM!The struct definition correctly defines the fields for
Config, and theValidatemethod ensures the configuration is valid.m2m-oauth-server/pb/service.proto (7)
31-61: LGTM!The message definition correctly defines the structure of a token and includes all necessary fields.
65-69: LGTM!The message definition correctly defines the structure of a request to get tokens and includes all necessary fields.
71-73: LGTM!The message definition correctly defines the structure of a request to blacklist tokens and includes all necessary fields.
75-77: LGTM!The message definition correctly defines the structure of a response to a blacklist tokens request and includes all necessary fields.
79-89: LGTM!The message definition correctly defines the structure of a request to create a token and includes all necessary fields.
91-96: LGTM!The message definition correctly defines the structure of a response to a create token request and includes all necessary fields.
98-130: LGTM!The service definition correctly defines the RPC methods for the M2M OAuth service and includes the necessary options for HTTP and OpenAPI support.
m2m-oauth-server/service/http/postToken.go (5)
30-52: Verify handling ofTimeToLivevalue.Ensure that the
TimeToLivevalue is correctly parsed and handled. Consider adding error handling for cases wherettlis not a valid integer.- if ttl == "" { - return - } - if ttlVal, err := strconv.ParseInt(ttl, 10, 64); err == nil { - createTokenRequest.TimeToLive = ttlVal - } + if ttl != "" { + if ttlVal, err := strconv.ParseInt(ttl, 10, 64); err != nil { + serverMux.WriteError(w, pkgGrpc.ForwardErrorf(codes.InvalidArgument, "invalid ttl value: %v", err)) + return + } + createTokenRequest.TimeToLive = ttlVal + }
54-70: LGTM!The function correctly converts JSON data to a
CreateTokenRequestobject.
72-112: Verify error handling and response writing.Ensure that all potential errors are correctly handled and that the response is written appropriately. Consider adding more detailed error messages and handling edge cases.
- if err != nil { - serverMux.WriteError(w, pkgGrpc.ForwardErrorf(codes.InvalidArgument, cannotCreateTokenFmt, err)) - return - } + if err != nil { + serverMux.WriteError(w, pkgGrpc.ForwardErrorf(codes.Internal, cannotCreateTokenFmt, err)) + return + } - if err = jsonResponseWriter(w, resp); err != nil { - log.Errorf("failed to write response: %v", err) - } + if err := jsonResponseWriter(w, resp); err != nil { + serverMux.WriteError(w, pkgGrpc.ForwardErrorf(codes.Internal, "failed to write response: %v", err)) + log.Errorf("failed to write response: %v", err) + }
18-28: LGTM!The
postRequeststruct is correctly defined with appropriate JSON tags.
110-112: Verify implementation ofjsonResponseWriter.Ensure that the
jsonResponseWriterfunction correctly handles JSON responses and potential errors.m2m-oauth-server/pb/token.go (6)
32-47: LGTM!The function correctly converts a
Tokenobject to a map.
49-66: LGTM!The function correctly replaces string values with int64 values in a map.
68-78: LGTM!The function correctly replaces int64 values with string values in a map.
80-102: LGTM!The function correctly converts a
Tokenobject to a BSON map.
104-117: LGTM!The function correctly converts a map to a
Tokenobject.
119-135: LGTM!The function correctly converts a BSON map to a
Tokenobject.m2m-oauth-server/store/mongodb/tokens.go (6)
17-40: LGTM!The function correctly handles token creation in the MongoDB store.
42-77: LGTM!The function correctly creates a MongoDB filter for retrieving tokens.
80-100: LGTM!The function correctly processes a MongoDB cursor and applies a processing function to each document.
102-116: LGTM!The function correctly retrieves tokens from the MongoDB store.
118-126: LGTM!The function correctly deletes expired and blacklisted tokens from the MongoDB store.
128-180: LGTM!The function correctly blacklists tokens in the MongoDB store.
pkg/security/jwt/claims.go (2)
25-25: LGTM!The change to the
ClaimNameconstant improves clarity and consistency in naming conventions.
27-27: LGTM!The addition of the
ErrOwnerClaimInvaliderror variable improves error handling.m2m-oauth-server/service/grpc/server.go (10)
32-38: LGTM!The function
NewM2MOAuthServerServercorrectly initializes a new instance of theM2MOAuthServiceServerstruct.
40-46: LGTM!The function
getOwnercorrectly retrieves the owner from the token metadata and handles errors appropriately.
48-53: LGTM!The function
getGRPCErrorCodecorrectly maps errors to appropriate gRPC error codes.
55-57: LGTM!The function
errCannotCreateConfigurationcorrectly wraps the provided error with a specific message.
59-61: LGTM!The function
errCannotCreateTokencorrectly wraps the provided error with a specific message.
63-121: LGTM!The function
CreateTokencomprehensively handles the creation of a token, including validation, access token generation, and storage. Error handling is appropriately implemented.
128-139: LGTM!The function
GetTokenscorrectly retrieves tokens for a specific owner and sends them to the client using a stream. Error handling is appropriately implemented.
146-155: LGTM!The function
BlacklistTokenscorrectly handles the blacklisting of tokens for a specific owner and handles errors appropriately.
158-160: LGTM!The function
GetJWKcorrectly retrieves the JWK key from the signer.
162-164: LGTM!The function
GetDomaincorrectly retrieves the domain from the signer configuration.m2m-oauth-server/service/service.go (5)
36-61: LGTM!The function
createStorecorrectly handles the creation of a MongoDB store, including error handling and scheduling for token cleanup.
64-82: LGTM!The function
newHttpServicecorrectly handles the creation of an HTTP service, including validation and configuration. Error handling is appropriately implemented.
84-94: LGTM!The function
newGrpcServicecorrectly handles the creation of a gRPC service, including validation and configuration. Error handling is appropriately implemented.
96-154: LGTM!The function
Newcorrectly handles the initialization of the service, including creating the store, OAuth signer, HTTP service, and gRPC service. Error handling and cleanup functions are appropriately implemented.
157-158: LGTM!The function
SnippetServiceStorecorrectly provides access to the store.m2m-oauth-server/test/test.go (5)
36-44: LGTM!The declaration of
ServiceOAuthClienthas been correctly updated to useoauthsigner.Client.
47-57: LGTM!The declaration of
JWTPrivateKeyOAuthClienthas been correctly updated to useoauthsigner.Clientandoauthsigner.PrivateKeyJWTConfig.
60-62: LGTM!The declaration of
OAuthClientshas been correctly updated to useoauthsigner.OAuthClientsConfig.
Line range hint
69-87:
LGTM!The
MakeConfigfunction correctly updates the configurations to use the new types from theoauthsignerpackage. Error handling is appropriately implemented.
207-208: LGTM!The
GetAccessTokenfunction correctly updates the grant type to use the new type from theoauthsignerpackage.m2m-oauth-server/store/mongodb/tokens_test.go (3)
14-120: LGTM!The test function
TestGetTokensis well-structured and comprehensive, covering various scenarios for token retrieval. Error handling and assertions are appropriately implemented.
122-213: LGTM!The test function
TestBlacklistTokensis well-structured and comprehensive, covering the blacklisting of tokens and verifying the results. Error handling and assertions are appropriately implemented.
215-293: LGTM!The test function
TestDeleteTokensis well-structured and comprehensive, covering the deletion of tokens and verifying the remaining tokens. Error handling and assertions are appropriately implemented.m2m-oauth-server/pb/README.md (5)
28-37: LGTM!The
BlacklistTokensRequestmessage is correctly defined.
43-52: LGTM!The
BlacklistTokensResponsemessage is correctly defined.
58-75: LGTM!The
CreateTokenRequestmessage is correctly defined.
81-93: LGTM!The
CreateTokenResponsemessage is correctly defined.
12-12: Possible typo: repeated word "Token".There seems to be a repeated word "Token" in the Table of Contents.
- - [Token](#m2moauthserver-pb-Token) - [Token.BlackListed](#m2moauthserver-pb-Token-BlackListed) + - [Token](#m2moauthserver-pb-Token) + - [Token.BlackListed](#m2moauthserver-pb-Token-BlackListed)Likely invalid or redundant comment.
Tools
LanguageTool
[duplication] ~12-~12: Possible typo: you repeated a word
Context: ...oauthserver-pb-GetTokensRequest) - Token - [Token.BlackListed](#m2moauthserver-pb-Token-B...(ENGLISH_WORD_REPEAT_RULE)
Markdownlint
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
m2m-oauth-server/service/grpc/token.go (12)
17-19: LGTM!The
setKeyErrorfunction is correctly implemented.
21-23: LGTM!The
setKeyErrorExtfunction is correctly implemented.
75-83: LGTM!The
getSubjectfunction is correctly implemented.
85-90: LGTM!The
setDeviceIDClaimfunction is correctly implemented.
92-96: LGTM!The
setOwnerClaimfunction is correctly implemented.
99-103: LGTM!The
setNamefunction is correctly implemented.
106-110: LGTM!The
setOriginTokenClaimsfunction is correctly implemented.
113-123: LGTM!The
getExpirationTimefunction is correctly implemented.
125-135: LGTM!The
generateAccessTokenfunction is correctly implemented.
154-167: LGTM!The
sliceContainsfunction is correctly implemented.
169-189: LGTM!The
validateTokenRequestfunction is correctly implemented.
192-199: LGTM!The
validateClientfunction is correctly implemented.m2m-oauth-server/service/http/getTokens_test.go (1)
57-72: Avoid usingdeferinside a loop.Using
deferinside a loop can lead to resource leaks. Consider closing the response body explicitly inside the loop.- defer func() { - _ = resp.Body.Close() - }() + _ = resp.Body.Close()Likely invalid or redundant comment.
test/config/config.go (1)
Line range hint
1-47:
LGTM!The new constant
M2M_OAUTH_SERVER_HOSTis correctly definedMakefile (1)
47-47: LGTM! Ensure the new directory exists.The change to the
SUBDIRSvariable is approved. Verify that them2m-oauth-serverdirectory exists and is correctly configured for the build process.Verification successful
Verification Successful: Directory Exists
The
m2m-oauth-serverdirectory exists, confirming the validity of the change in theSUBDIRSvariable. The update to theMakefileis approved.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the `m2m-oauth-server` directory. # Test: Check if the directory exists. Expect: The directory should exist. if [ -d "m2m-oauth-server" ]; then echo "Directory exists" else echo "Directory does not exist" exit 1 fiLength of output: 72
m2m-oauth-server/pb/doc.html (5)
1-16: LGTM! The head section is well-formed.The head section includes necessary metadata and styling.
17-221: LGTM! The body section is well-structured.The body section provides comprehensive documentation with a clear table of contents and detailed protocol information.
235-563: LGTM! The tables are well-formatted.The tables provide clear information about each field in the protocol messages.
575-653: LGTM! The methods section is clear.The methods section provides necessary details about each method and its HTTP binding.
658-846: LGTM! The scalar value types section is informative.The scalar value types section provides a useful table mapping .proto types to various programming languages.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (11)
charts/plgd-hub/templates/m2m-oauth-server/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/grpc-ingress.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/grpc-service.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/http-ingress.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/http-service.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/service-crt.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/snippet-service/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/snippet-service/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/values.yamlis excluded by!**/*.yamltest/helm/mock.plgd.cloud.yamlis excluded by!**/*.yaml
Files selected for processing (1)
- charts/plgd-hub/templates/m2m-oauth-server/_helpers.tpl (3 hunks)
Additional comments not posted (3)
charts/plgd-hub/templates/m2m-oauth-server/_helpers.tpl (3)
20-20: Verify the correctness of the TLS certificate file reference change.Ensure that the reference to the gRPC TLS certificate file is intentional and aligns with the overall refactor to support gRPC.
Line range hint
41-41:
LGTM!The reference to the private key file for the OAuth signer is correct.
Line range hint
49-49:
LGTM!The condition to check if the M2M OAuth server is enabled and if the private key secret is enabled is correct.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (4)
m2m-oauth-server/pb/service.proto (3)
75-77: Field descriptions are missing in theBlacklistTokensResponsemessage.Please add clear and complete descriptions for the fields in the
BlacklistTokensResponsemessage to improve documentation and maintainability.
- File:
m2m-oauth-server/pb/service.proto- Lines: 75-77
Analysis chain
LGTM! Verify field descriptions for clarity and completeness.
The message structure is approved.
However, ensure that all field descriptions are clear and complete.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify field descriptions in the BlacklistTokensResponse message. # Test: Search for the BlacklistTokensResponse message definition. Expect: Clear and complete field descriptions. rg --type proto -A 5 $'message BlacklistTokensResponse'Length of output: 86
Script:
#!/bin/bash # Locate all .proto files and then search for the BlacklistTokensResponse message definition within those files. # Find all .proto files fd --extension proto > proto_files.txt # Search for the BlacklistTokensResponse message definition within the located .proto files xargs -a proto_files.txt rg 'message BlacklistTokensResponse' -A 5Length of output: 395
100-105: Field descriptions are missing in theCreateTokenResponsemessage.The structure of the
CreateTokenResponsemessage is correct, but the fields lack descriptions. Please add clear and complete descriptions for each field to ensure proper understanding and documentation.
access_tokentoken_typeexpires_inscopeAnalysis chain
LGTM! Verify field descriptions for clarity and completeness.
The message structure is approved.
However, ensure that all field descriptions are clear and complete.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify field descriptions in the CreateTokenResponse message. # Test: Search for the CreateTokenResponse message definition. Expect: Clear and complete field descriptions. rg --type proto -A 5 $'message CreateTokenResponse'Length of output: 82
Script:
#!/bin/bash # Locate the service.proto file and search for the CreateTokenResponse message definition along with its field descriptions. # Find the service.proto file fd service.proto # Search for the CreateTokenResponse message definition in the located service.proto file rg 'message CreateTokenResponse' $(fd service.proto)Length of output: 617
Script:
#!/bin/bash # Search for the CreateTokenResponse message definition in the m2m-oauth-server/pb/service.proto file and include surrounding lines for context. rg 'message CreateTokenResponse' -A 10 m2m-oauth-server/pb/service.protoLength of output: 367
65-69: Add field descriptions toGetTokensRequestmessage.The
GetTokensRequestmessage inm2m-oauth-server/pb/service.protolacks field descriptions. Please add comments to clarify the purpose and usage of each field:
id_filteraudience_filterinclude_blacklistedThis will improve the readability and maintainability of the code.
Analysis chain
LGTM! Verify field descriptions for clarity and completeness.
The message structure is approved.
However, ensure that all field descriptions are clear and complete.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify field descriptions in the GetTokensRequest message. # Test: Search for the GetTokensRequest message definition. Expect: Clear and complete field descriptions. rg --type proto -A 5 $'message GetTokensRequest'Length of output: 79
Script:
#!/bin/bash # Description: Verify field descriptions in the GetTokensRequest message. # Test: Search for the GetTokensRequest message definition. Expect: Clear and complete field descriptions. rg -A 5 'message GetTokensRequest'Length of output: 374
m2m-oauth-server/pb/README.md (1)
6-17: Fix unordered list indentation.The unordered list indentation is inconsistent.
- 6-17: Expected: 2; Actual: 4 + 6-17: Expected: 2; Actual: 4Tools
LanguageTool
[duplication] ~12-~12: Possible typo: you repeated a word
Context: ...oauthserver-pb-GetTokensRequest) - Token - [Token.BlackListed](#m2moauthserver-pb-Token-B...(ENGLISH_WORD_REPEAT_RULE)
Markdownlint
7-7: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
8-8: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
9-9: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
10-10: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
11-11: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
13-13: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
15-15: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (7)
charts/plgd-hub/templates/m2m-oauth-server/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/http-ingress.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/http-service.yamlis excluded by!**/*.yamlm2m-oauth-server/pb/service.pb.gois excluded by!**/*.pb.go,!**/*.pb.gom2m-oauth-server/pb/service.swagger.jsonis excluded by!**/*.jsonm2m-oauth-server/swagger.yamlis excluded by!**/*.yaml
Files selected for processing (13)
- m2m-oauth-server/Makefile (3 hunks)
- m2m-oauth-server/pb/README.md (1 hunks)
- m2m-oauth-server/pb/doc.html (1 hunks)
- m2m-oauth-server/pb/service.proto (1 hunks)
- m2m-oauth-server/service/grpc/server.go (1 hunks)
- m2m-oauth-server/service/grpc/token.go (1 hunks)
- m2m-oauth-server/service/http/postToken.go (1 hunks)
- m2m-oauth-server/store/mongodb/tokens.go (1 hunks)
- m2m-oauth-server/store/mongodb/tokens_test.go (1 hunks)
- m2m-oauth-server/test/test.go (4 hunks)
- m2m-oauth-server/uri/uri.go (1 hunks)
- pkg/security/jwt/validator/validator.go (3 hunks)
- pkg/time/unixnano.go (1 hunks)
Files skipped from review as they are similar to previous changes (10)
- m2m-oauth-server/Makefile
- m2m-oauth-server/pb/doc.html
- m2m-oauth-server/service/grpc/server.go
- m2m-oauth-server/service/grpc/token.go
- m2m-oauth-server/service/http/postToken.go
- m2m-oauth-server/store/mongodb/tokens.go
- m2m-oauth-server/store/mongodb/tokens_test.go
- m2m-oauth-server/test/test.go
- m2m-oauth-server/uri/uri.go
- pkg/security/jwt/validator/validator.go
Additional context used
LanguageTool
m2m-oauth-server/pb/README.md
[duplication] ~12-~12: Possible typo: you repeated a word
Context: ...oauthserver-pb-GetTokensRequest) - Token - [Token.BlackListed](#m2moauthserver-pb-Token-B...(ENGLISH_WORD_REPEAT_RULE)
Markdownlint
m2m-oauth-server/pb/README.md
7-7: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
8-8: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
9-9: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
10-10: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
11-11: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
13-13: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
15-15: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
135-135: null
Link fragments should be valid(MD051, link-fragments)
Additional comments not posted (8)
pkg/time/unixnano.go (1)
19-25: LGTM! Ensure consistency withUnixNano.The code changes are approved.
However, ensure that the logic in
UnixSecis consistent withUnixNano.Verification successful
I will generate a new shell script to search for any function related to Unix time in nanoseconds across the Go files in the repository.
The logic in
UnixSecis consistent withUnixNano.Both functions check if the
time.Timeobject is zero and return the corresponding Unix time (in seconds or nanoseconds).
UnixSec:func UnixSec(t time.Time) int64 { v := int64(0) if (!t.IsZero()) { v = t.Unix() } return v }
UnixNano:func UnixNano(t time.Time) int64 { v := int64(0) if (!t.IsZero()) { v = t.UnixNano() } return v }Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency between UnixSec and UnixNano functions. # Test: Search for the UnixNano function definition. Expect: Consistent logic. rg --type go -A 5 $'func UnixNano(t time.Time) int64'Length of output: 54
Script:
#!/bin/bash # Description: Search for functions related to Unix time in nanoseconds. # Test: Search for any function containing "UnixNano" or similar in its name. rg --type go -A 5 'func.*UnixNano'Length of output: 262
m2m-oauth-server/pb/service.proto (4)
71-73: LGTM! Verify field descriptions for clarity and completeness.The message structure is approved.
However, ensure that all field descriptions are clear and complete.
79-98: LGTM! Verify field descriptions for clarity and completeness.The message structure is approved.
However, ensure that all field descriptions are clear and complete.
Verification successful
Field descriptions are clear and complete.
The field descriptions in the
CreateTokenRequestmessage are clear and provide necessary information about each field.
client_id: Client IDclient_secret: Client Secretaudience: Requested token Audiencescope: Requested token scopesexpiration: Requested token expiration in Unix timestamp secondsclient_assertion_type: Client assertion typeclient_assertion: Client assertiontoken_name: Token namegrant_type: Grant typeScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify field descriptions in the CreateTokenRequest message. # Test: Search for the CreateTokenRequest message definition. Expect: Clear and complete field descriptions. rg --type proto -A 10 $'message CreateTokenRequest'Length of output: 82
Script:
#!/bin/bash # Description: Verify field descriptions in the CreateTokenRequest message. # Find all .proto files and search for the CreateTokenRequest message definition. fd -e proto -x rg 'message CreateTokenRequest' -A 10 {}Length of output: 375
Script:
#!/bin/bash # Description: Verify field descriptions in the CreateTokenRequest message. # Find all .proto files and search for the CreateTokenRequest message definition with more context lines. fd -e proto -x rg 'message CreateTokenRequest' -A 20 {}Length of output: 576
107-139: LGTM! Verify method descriptions for clarity and completeness.The service structure is approved.
However, ensure that all method descriptions are clear and complete.
Verification successful
Let's correct the approach to verify the method descriptions in the
M2MOAuthServiceservice.
Let's adjust the script to capture more lines following the service definition to ensure we get the complete method descriptions.
Method descriptions are clear and complete.
The service structure and method descriptions in the
M2MOAuthServiceservice are well-defined and provide sufficient clarity.
CreateToken: Creates a new token.GetTokens: Returns all tokens of the owner.BlacklistTokens: Blacklists/revokes tokens.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify method descriptions in the M2MOAuthService service. # Test: Search for the M2MOAuthService service definition. Expect: Clear and complete method descriptions. rg --type proto -A 10 $'service M2MOAuthService'Length of output: 79
Script:
#!/bin/bash # Description: Verify method descriptions in the M2MOAuthService service. # Locate all .proto files and search for the M2MOAuthService service definition. fd --type f --extension proto --exec rg 'service M2MOAuthService' -A 10Length of output: 414
Script:
#!/bin/bash # Description: Verify method descriptions in the M2MOAuthService service. # Locate all .proto files and search for the M2MOAuthService service definition, capturing more lines for complete method descriptions. fd --type f --extension proto --exec rg 'service M2MOAuthService' -A 50Length of output: 1052
31-61: LGTM! Verify field descriptions for clarity and completeness.The message structure is approved.
However, ensure that all field descriptions are clear and complete.
Verification successful
Field descriptions in the
Tokenmessage are clear and complete.The message structure and field descriptions for the
Tokenmessage have been verified and are satisfactory.
- The message structure is well-defined.
- Field descriptions are clear and provide necessary details.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify field descriptions in the Token message. # Test: Search for the Token message definition. Expect: Clear and complete field descriptions. rg --type proto -A 10 $'message Token'Length of output: 69
Script:
#!/bin/bash # Locate all .proto files and search for the Token message definition within those files. fd -e proto -x rg 'message Token' -A 20 {}Length of output: 666
Script:
#!/bin/bash # Locate all .proto files and search for the Token message definition within those files with a larger context. fd -e proto -x rg 'message Token' -A 50 {}Length of output: 1323
m2m-oauth-server/pb/README.md (3)
28-40: LGTM! Verify table formatting for completeness.The section structure is approved.
However, ensure that the table formatting is complete.
Verification successful
Table formatting is complete for the
BlacklistTokensRequestsection.
- The table includes all necessary headers and fields.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify table formatting in the BlacklistTokensRequest section. # Test: Search for the BlacklistTokensRequest section. Expect: Complete table formatting. rg --type md -A 10 $'BlacklistTokensRequest'Length of output: 2328
180-199: LGTM! Verify table formatting for completeness.The section structure is approved.
However, ensure that the table formatting is complete.
Verification successful
Table formatting verified and complete.
The
Scalar Value Typessection in them2m-oauth-server/pb/README.mdfile is correctly formatted and complete.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify table formatting in the Scalar Value Types section. # Test: Search for the Scalar Value Types section. Expect: Complete table formatting. rg --type md -A 10 $'Scalar Value Types'Length of output: 11225
59-75: LGTM! Verify table formatting for completeness.The section structure is approved.
However, ensure that the table formatting is complete.
Verification successful
Verify the complete table formatting for
CreateTokenRequest.The section structure is approved, but we need to ensure the table formatting is complete.
The table formatting for
CreateTokenRequestis complete and correctly structured.The section structure and table formatting for
CreateTokenRequestinm2m-oauth-server/pb/README.mdare verified and found to be complete.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify table formatting in the CreateTokenRequest section. # Test: Search for the CreateTokenRequest section. Expect: Complete table formatting. rg --type md -A 10 $'CreateTokenRequest'Length of output: 2599
Script:
#!/bin/bash # Description: Verify complete table formatting in the CreateTokenRequest section. # Test: Search for the CreateTokenRequest section and display more lines to capture the entire table. rg --type md -A 20 $'CreateTokenRequest'Length of output: 5634
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- m2m-oauth-server/pb/token.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- m2m-oauth-server/pb/token.go
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
m2m-oauth-server/pb/service.pb.gois excluded by!**/*.pb.go,!**/*.pb.gom2m-oauth-server/pb/service.swagger.jsonis excluded by!**/*.jsonm2m-oauth-server/swagger.yamlis excluded by!**/*.yaml
Files selected for processing (5)
- m2m-oauth-server/pb/README.md (1 hunks)
- m2m-oauth-server/pb/doc.html (1 hunks)
- m2m-oauth-server/pb/service.proto (1 hunks)
- m2m-oauth-server/service/grpc/server.go (1 hunks)
- m2m-oauth-server/service/grpc/token.go (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- m2m-oauth-server/pb/doc.html
- m2m-oauth-server/pb/service.proto
- m2m-oauth-server/service/grpc/server.go
- m2m-oauth-server/service/grpc/token.go
Additional context used
Markdownlint
m2m-oauth-server/pb/README.md
7-7: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
8-8: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
9-9: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
10-10: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
11-11: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
13-13: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
15-15: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
135-135: null
Link fragments should be valid(MD051, link-fragments)
Additional comments not posted (8)
m2m-oauth-server/pb/README.md (8)
28-37: LGTM!The
BlacklistTokensRequestsection is well-defined and correctly formatted.
43-52: LGTM!The
BlacklistTokensResponsesection is well-defined and correctly formatted.
59-75: LGTM!The
CreateTokenRequestsection is well-defined and correctly formatted.
82-93: LGTM!The
CreateTokenResponsesection is well-defined and correctly formatted.
100-110: LGTM!The
GetTokensRequestsection is well-defined and correctly formatted.
167-176: LGTM!The
M2MOAuthServicesection is well-defined and correctly formatted.
181-199: LGTM!The
Scalar Value Typessection is well-defined and correctly formatted.
117-154: LGTM! But verify the link fragment forgoogle.protobuf.Value.The
Tokensection is well-defined and correctly formatted.However, ensure that the link fragment for
google.protobuf.Valueis valid.Verification successful
Verified: The link fragment for
google.protobuf.Valueis valid.The reference to
google.protobuf.Valuein theREADME.mdfile is correct and points to a valid definition in theservice.protofile.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the link fragment for `google.protobuf.Value`. # Test: Search for the definition of `google.protobuf.Value`. Expect: Valid link fragment. rg --type proto --pattern 'google.protobuf.Value'Length of output: 461
Script:
#!/bin/bash # Description: Verify the link fragment for `google.protobuf.Value`. # Test: Search for the definition of `google.protobuf.Value`. Expect: Valid link fragment. rg 'google.protobuf.Value' --type protoLength of output: 69
Script:
#!/bin/bash # Description: Verify the link fragment for `google.protobuf.Value`. # Test: Search for the definition of `google.protobuf.Value`. Expect: Valid link fragment. rg 'google.protobuf.Value'Length of output: 581
Tools
Markdownlint
135-135: null
Link fragments should be valid(MD051, link-fragments)
|
@coderabbitai generate a description for this pullrequest about the managing m2m-oauth-server tokens feature targeting to not plgd developers. |
|
@jkralik: Here is a description for the pull request about managing M2M OAuth server tokens, targeting non-plgd developers: Managing M2M OAuth Server TokensThis pull request introduces comprehensive functionalities for managing tokens in the M2M OAuth server, enhancing the server's ability to handle token creation, retrieval, blacklisting, and deletion. The key features are:
These enhancements make the M2M OAuth server more robust and secure, providing essential features for managing API access tokens in a machine-to-machine context. Actions performedComments resolved. |
jkralik
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (14)
charts/plgd-hub/templates/certificate-authority/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/certificate-authority/service-crt.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/coap-gateway/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/grpc-gateway/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/http-gateway/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/identity-store/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/service-crt.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/mock-oauth-server/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/resource-aggregate/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/resource-directory/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/snippet-service/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/snippet-service/service-crt.yamlis excluded by!**/*.yaml
Files selected for processing (1)
- charts/plgd-hub/templates/_helpers.tpl (2 hunks)
Files skipped from review due to trivial changes (1)
- charts/plgd-hub/templates/_helpers.tpl
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
charts/plgd-hub/templates/snippet-service/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/values.yamlis excluded by!**/*.yamltest/helm/mock.plgd.cloud.yamlis excluded by!**/*.yaml
Files selected for processing (2)
- Makefile (1 hunks)
- test/config/config.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- Makefile
- test/config/config.go
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
m2m-oauth-server/pb/README.md (2)
6-17: Fix unordered list indentation.The unordered list indentation should be corrected to follow Markdown best practices.
- - [m2m-oauth-server/pb/service.proto](#m2m-oauth-server_pb_service-proto) - - [BlacklistTokensRequest](#m2moauthserver-pb-BlacklistTokensRequest) - - [BlacklistTokensResponse](#m2moauthserver-pb-BlacklistTokensResponse) - - [CreateTokenRequest](#m2moauthserver-pb-CreateTokenRequest) - - [CreateTokenResponse](#m2moauthserver-pb-CreateTokenResponse) - - [GetTokensRequest](#m2moauthserver-pb-GetTokensRequest) - - [Token](#m2moauthserver-pb-Token) - - [Token.BlackListed](#m2moauthserver-pb-Token-BlackListed) - - [M2MOAuthService](#m2moauthserver-pb-M2MOAuthService) - - [Scalar Value Types](#scalar-value-types) + - [m2m-oauth-server/pb/service.proto](#m2m-oauth-server_pb_service-proto) + - [BlacklistTokensRequest](#m2moauthserver-pb-BlacklistTokensRequest) + - [BlacklistTokensResponse](#m2moauthserver-pb-BlacklistTokensResponse) + - [CreateTokenRequest](#m2moauthserver-pb-CreateTokenRequest) + - [CreateTokenResponse](#m2moauthserver-pb-CreateTokenResponse) + - [GetTokensRequest](#m2moauthserver-pb-GetTokensRequest) + - [Token](#m2moauthserver-pb-Token) + - [Token.BlackListed](#m2moauthserver-pb-Token-BlackListed) + - [M2MOAuthService](#m2moauthserver-pb-M2MOAuthService) + - [Scalar Value Types](#scalar-value-types)Tools
LanguageTool
[duplication] ~12-~12: Possible typo: you repeated a word
Context: ...oauthserver-pb-GetTokensRequest) - Token - [Token.BlackListed](#m2moauthserver-pb-Token-B...(ENGLISH_WORD_REPEAT_RULE)
Markdownlint
7-7: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
8-8: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
9-9: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
10-10: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
11-11: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
13-13: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
15-15: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
116-136: Fix table formatting for theidfield.The table for the
idfield is missing a trailing pipe.- | id | [string](#string) | | Token ID / jti + | id | [string](#string) | | Token ID / jti |Tools
Markdownlint
135-135: null
Link fragments should be valid(MD051, link-fragments)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (8)
charts/plgd-hub/templates/certificate-authority/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/snippet-service/config.yamlis excluded by!**/*.yamlgrpc-gateway/pb/hubConfiguration.pb.gois excluded by!**/*.pb.go,!**/*.pb.gom2m-oauth-server/pb/service.pb.gois excluded by!**/*.pb.go,!**/*.pb.gom2m-oauth-server/pb/service.swagger.jsonis excluded by!**/*.jsonm2m-oauth-server/swagger.yamlis excluded by!**/*.yamlsnippet-service/pb/service.pb.gois excluded by!**/*.pb.go,!**/*.pb.go
Files selected for processing (20)
- m2m-oauth-server/oauthSigner/oauthSigner.go (1 hunks)
- m2m-oauth-server/pb/README.md (1 hunks)
- m2m-oauth-server/pb/doc.html (1 hunks)
- m2m-oauth-server/pb/service.proto (1 hunks)
- m2m-oauth-server/pb/tags.go (1 hunks)
- m2m-oauth-server/pb/token.go (1 hunks)
- m2m-oauth-server/service/grpc/server.go (1 hunks)
- m2m-oauth-server/service/grpc/service.go (1 hunks)
- m2m-oauth-server/service/grpc/token.go (1 hunks)
- m2m-oauth-server/service/http/postToken_test.go (3 hunks)
- m2m-oauth-server/service/http/requestHandler.go (1 hunks)
- m2m-oauth-server/service/service.go (1 hunks)
- m2m-oauth-server/store/mongodb/tokens.go (1 hunks)
- m2m-oauth-server/store/mongodb/tokens_test.go (1 hunks)
- m2m-oauth-server/store/store.go (1 hunks)
- m2m-oauth-server/test/test.go (7 hunks)
- pkg/mongodb/marshal.go (2 hunks)
- pkg/mongodb/marshal_test.go (1 hunks)
- snippet-service/pb/appliedConfiguration.go (2 hunks)
- snippet-service/store/appliedConfiguration.go (1 hunks)
Files skipped from review due to trivial changes (3)
- m2m-oauth-server/pb/doc.html
- m2m-oauth-server/pb/service.proto
- m2m-oauth-server/pb/tags.go
Files skipped from review as they are similar to previous changes (10)
- m2m-oauth-server/oauthSigner/oauthSigner.go
- m2m-oauth-server/pb/token.go
- m2m-oauth-server/service/grpc/service.go
- m2m-oauth-server/service/grpc/token.go
- m2m-oauth-server/service/http/postToken_test.go
- m2m-oauth-server/service/http/requestHandler.go
- m2m-oauth-server/service/service.go
- m2m-oauth-server/store/mongodb/tokens.go
- m2m-oauth-server/store/mongodb/tokens_test.go
- m2m-oauth-server/test/test.go
Additional context used
LanguageTool
m2m-oauth-server/pb/README.md
[duplication] ~12-~12: Possible typo: you repeated a word
Context: ...oauthserver-pb-GetTokensRequest) - Token - [Token.BlackListed](#m2moauthserver-pb-Token-B...(ENGLISH_WORD_REPEAT_RULE)
Markdownlint
m2m-oauth-server/pb/README.md
7-7: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
8-8: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
9-9: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
10-10: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
11-11: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
13-13: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
15-15: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
135-135: null
Link fragments should be valid(MD051, link-fragments)
Additional comments not posted (44)
m2m-oauth-server/store/store.go (5)
1-10: LGTM!The package declaration and imports are appropriate for the functionalities described.
12-15: LGTM!The
Iteratorinterface is well-defined for iterating over generic types.
17-28: LGTM!The type definitions and error variables are clear and useful for token management functionalities.
30-44: LGTM!The
MongoIteratorstruct and methods correctly implement theIteratorinterface for MongoDB results.
46-58: LGTM!The
Storeinterface is well-defined for token management functionalities.snippet-service/store/appliedConfiguration.go (1)
47-63: LGTM!The changes to the
UnmarshalBSONmethod enhance error handling and improve the robustness of the unmarshalling process.snippet-service/pb/appliedConfiguration.go (2)
95-99: LGTM!The changes to the
jsonToBSONTagmethod inAppliedConfiguration_Resourceimprove error handling by properly reporting conversion errors.
139-149: LGTM!The changes to the
jsonToBSONTagmethod inAppliedConfigurationimprove error handling by properly reporting conversion errors for multiple fields.pkg/mongodb/marshal_test.go (11)
25-32: LGTM!The test case correctly checks for an error when the path is empty.
33-40: LGTM!The test case correctly checks for an error when the path is invalid.
41-48: LGTM!The test case correctly checks for the conversion of a direct string value to int64.
49-60: LGTM!The test case correctly checks for the conversion of specific string values in an array to int64.
61-72: LGTM!The test case correctly checks for the conversion of a string value in a map to int64.
73-87: LGTM!The test case correctly checks for the conversion of specific string values in an array within a map to int64.
88-103: LGTM!The test case correctly checks for the conversion of a string value in a nested map to int64.
104-129: LGTM!The test case correctly checks for the conversion of specific string values in nested maps within an array to int64.
130-155: LGTM!The test case correctly checks for the conversion of all string values in nested maps within an array to int64.
156-188: LGTM!The test case correctly checks for the conversion of all string values in nested maps within an array to int64, while permitting missing paths.
189-204: LGTM!The test case correctly checks for the conversion of all string values in an array within a map to int64.
m2m-oauth-server/service/grpc/server.go (10)
32-38: LGTM!The function correctly initializes a new
M2MOAuthServiceServerwith the provided store, signer, and logger.
40-46: LGTM!The function correctly retrieves the owner from the token metadata in the context.
48-53: LGTM!The function correctly maps store errors to gRPC error codes.
55-57: LGTM!The function correctly wraps an error with a specific message for configuration creation.
59-61: LGTM!The function correctly wraps an error with a specific message for token creation.
63-123: LGTM!The function correctly handles the creation of a new token, including validation and storage.
126-128: LGTM!The function correctly wraps an error with a specific message for getting tokens.
130-141: LGTM!The function correctly handles the retrieval of tokens.
144-146: LGTM!The function correctly wraps an error with a specific message for blacklisting tokens.
148-157: LGTM!The function correctly handles the blacklisting of tokens.
pkg/mongodb/marshal.go (8)
17-17: LGTM!The change to the
updateJSONtype improves error handling by allowing the function to return an error.
27-36: LGTM!The changes to the
ConvertStringValueToInt64function enhance its usability by providing more control over how missing paths are handled and improving feedback for callers.
38-58: LGTM!The function correctly handles slices during the conversion process and includes error handling.
60-94: LGTM!The function correctly finds parent elements in the JSON structure and includes error handling.
98-107: LGTM!The function correctly splits a path into segments and cleans up the segments.
109-128: LGTM!The function correctly sets a value in a map during the conversion process and includes error handling.
130-147: LGTM!The function correctly sets a value in a slice during the conversion process and includes error handling.
149-165: LGTM!The function correctly sets values in a slice during the conversion process and includes error handling.
m2m-oauth-server/pb/README.md (7)
28-36: LGTM!The documentation for
BlacklistTokensRequestis complete and correctly formatted.
43-51: LGTM!The documentation for
BlacklistTokensResponseis complete and correctly formatted.
58-74: LGTM!The documentation for
CreateTokenRequestis complete and correctly formatted.
81-92: LGTM!The documentation for
CreateTokenResponseis complete and correctly formatted.
100-109: LGTM!The documentation for
GetTokensRequestis complete and correctly formatted.
145-153: LGTM!The documentation for
Token.BlackListedis complete and correctly formatted.
181-199: LGTM!The documentation for scalar value types is complete and correctly formatted.
jkralik
force-pushed
the
jkralik/feature/m2m-oauth-server-blacklist
branch
from
64c288f to
6d6f503
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range, codebase verification and nitpick comments (2)
m2m-oauth-server/service/http/postToken.go (2)
60-63: Ensure proper handling of emptyScopefield.The
Scopefield is currently split into a slice of strings only if it is not empty. Consider adding validation or handling for empty or malformedScopefields to ensure robustness.
m2m-oauth-server/service/http/postToken.go:
- Line 60:
scope := req.Scope- Line 61-63:
if scope != "" { createTokenRequest.Scope = strings.Split(scope, " ") }Analysis chain
Ensure proper handling of empty
Scopefield.The
Scopefield is split into a slice of strings. Ensure that empty strings are handled correctly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `Scope` field is correctly handled when empty or malformed. # Test: Search for the function usage. Expect: Proper handling of empty or malformed `Scope` field. rg --type go -A 5 $'jsonToCreateTokenRequest'Length of output: 1041
Script:
#!/bin/bash # Description: Verify that the `Scope` field is correctly handled when empty or malformed. # Search for the `Scope` field usage within the `jsonToCreateTokenRequest` function. rg --type go -A 10 'Scope' m2m-oauth-server/service/http/postToken.goLength of output: 1913
36-39: Ensure proper handling of emptyScopefield.The
postFormToCreateTokenRequestfunction does not handle cases where theScopefield might be empty or malformed. Consider adding a check to handle empty slices after splitting theScopefield.
- File:
m2m-oauth-server/service/http/postToken.go- Lines: 36-39
Analysis chain
Ensure proper handling of empty
Scopefield.The
Scopefield is split into a slice of strings. Ensure that empty strings are handled correctly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `Scope` field is correctly handled when empty or malformed. # Test: Search for the function usage. Expect: Proper handling of empty or malformed `Scope` field. rg --type go -A 5 $'postFormToCreateTokenRequest'Length of output: 1163
Script:
#!/bin/bash # Description: Extract the entire `postFormToCreateTokenRequest` function to verify the handling of the `Scope` field. # Extract the function definition and its body. ast-grep --lang go --pattern $'func postFormToCreateTokenRequest($_, $_) { $$$ }'Length of output: 2178
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (21)
charts/plgd-hub/templates/certificate-authority/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/grpc-ingress.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/grpc-service.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/http-ingress.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/http-service.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/service-crt.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/service.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/snippet-service/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/snippet-service/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/values.yamlis excluded by!**/*.yamlgrpc-gateway/pb/hubConfiguration.pb.gois excluded by!**/*.pb.go,!**/*.pb.gom2m-oauth-server/config.yamlis excluded by!**/*.yamlm2m-oauth-server/pb/service.pb.gois excluded by!**/*.pb.go,!**/*.pb.gom2m-oauth-server/pb/service.pb.gw.gois excluded by!**/*.pb.gw.go,!**/*.pb.gw.gom2m-oauth-server/pb/service.swagger.jsonis excluded by!**/*.jsonm2m-oauth-server/pb/service_grpc.pb.gois excluded by!**/*.pb.go,!**/*.pb.gom2m-oauth-server/swagger.yamlis excluded by!**/*.yamlsnippet-service/pb/service.pb.gois excluded by!**/*.pb.go,!**/*.pb.gotest/helm/mock.plgd.cloud.yamlis excluded by!**/*.yaml
Files selected for processing (54)
- Makefile (1 hunks)
- charts/plgd-hub/templates/m2m-oauth-server/_helpers.tpl (3 hunks)
- m2m-oauth-server/Makefile (3 hunks)
- m2m-oauth-server/oauthSigner/config.go (1 hunks)
- m2m-oauth-server/oauthSigner/loadKeys.go (1 hunks)
- m2m-oauth-server/oauthSigner/oauthSigner.go (1 hunks)
- m2m-oauth-server/pb/README.md (1 hunks)
- m2m-oauth-server/pb/doc.html (1 hunks)
- m2m-oauth-server/pb/service.proto (1 hunks)
- m2m-oauth-server/pb/tags.go (1 hunks)
- m2m-oauth-server/pb/token.go (1 hunks)
- m2m-oauth-server/service/config.go (2 hunks)
- m2m-oauth-server/service/expiredUpdatesChecker.go (1 hunks)
- m2m-oauth-server/service/grpc/config.go (1 hunks)
- m2m-oauth-server/service/grpc/server.go (1 hunks)
- m2m-oauth-server/service/grpc/service.go (1 hunks)
- m2m-oauth-server/service/grpc/token.go (1 hunks)
- m2m-oauth-server/service/http/config.go (1 hunks)
- m2m-oauth-server/service/http/createToken_test.go (1 hunks)
- m2m-oauth-server/service/http/getJWKs.go (2 hunks)
- m2m-oauth-server/service/http/getJWKs_test.go (1 hunks)
- m2m-oauth-server/service/http/getOpenIDConfiguration.go (2 hunks)
- m2m-oauth-server/service/http/getOpenIDConfiguration_test.go (1 hunks)
- m2m-oauth-server/service/http/getTokens_test.go (1 hunks)
- m2m-oauth-server/service/http/jsonWriter.go (1 hunks)
- m2m-oauth-server/service/http/postToken.go (1 hunks)
- m2m-oauth-server/service/http/postToken_test.go (3 hunks)
- m2m-oauth-server/service/http/requestHandler.go (1 hunks)
- m2m-oauth-server/service/http/service.go (1 hunks)
- m2m-oauth-server/service/service.go (1 hunks)
- m2m-oauth-server/service/service_test.go (1 hunks)
- m2m-oauth-server/store/config/config.go (1 hunks)
- m2m-oauth-server/store/config/config_test.go (1 hunks)
- m2m-oauth-server/store/cqldb/config.go (1 hunks)
- m2m-oauth-server/store/cqldb/store.go (1 hunks)
- m2m-oauth-server/store/cqldb/tokens.go (1 hunks)
- m2m-oauth-server/store/mongodb/config.go (1 hunks)
- m2m-oauth-server/store/mongodb/store.go (1 hunks)
- m2m-oauth-server/store/mongodb/tokens.go (1 hunks)
- m2m-oauth-server/store/mongodb/tokens_test.go (1 hunks)
- m2m-oauth-server/store/store.go (1 hunks)
- m2m-oauth-server/test/service.go (1 hunks)
- m2m-oauth-server/test/test.go (7 hunks)
- m2m-oauth-server/uri/uri.go (1 hunks)
- pkg/mongodb/marshal.go (2 hunks)
- pkg/mongodb/marshal_test.go (1 hunks)
- pkg/security/jwt/claims.go (1 hunks)
- pkg/security/jwt/validator/validator.go (3 hunks)
- pkg/security/openid/config.go (1 hunks)
- pkg/time/unixnano.go (1 hunks)
- snippet-service/pb/appliedConfiguration.go (2 hunks)
- snippet-service/service/http/service.go (2 hunks)
- snippet-service/store/appliedConfiguration.go (1 hunks)
- test/config/config.go (1 hunks)
Files skipped from review due to trivial changes (1)
- m2m-oauth-server/pb/tags.go
Files skipped from review as they are similar to previous changes (45)
- Makefile
- charts/plgd-hub/templates/m2m-oauth-server/_helpers.tpl
- m2m-oauth-server/Makefile
- m2m-oauth-server/oauthSigner/config.go
- m2m-oauth-server/oauthSigner/loadKeys.go
- m2m-oauth-server/oauthSigner/oauthSigner.go
- m2m-oauth-server/pb/doc.html
- m2m-oauth-server/pb/service.proto
- m2m-oauth-server/pb/token.go
- m2m-oauth-server/service/expiredUpdatesChecker.go
- m2m-oauth-server/service/grpc/config.go
- m2m-oauth-server/service/grpc/server.go
- m2m-oauth-server/service/grpc/service.go
- m2m-oauth-server/service/grpc/token.go
- m2m-oauth-server/service/http/config.go
- m2m-oauth-server/service/http/createToken_test.go
- m2m-oauth-server/service/http/getJWKs.go
- m2m-oauth-server/service/http/getJWKs_test.go
- m2m-oauth-server/service/http/getOpenIDConfiguration.go
- m2m-oauth-server/service/http/getOpenIDConfiguration_test.go
- m2m-oauth-server/service/http/jsonWriter.go
- m2m-oauth-server/service/http/requestHandler.go
- m2m-oauth-server/service/http/service.go
- m2m-oauth-server/service/service.go
- m2m-oauth-server/service/service_test.go
- m2m-oauth-server/store/config/config.go
- m2m-oauth-server/store/config/config_test.go
- m2m-oauth-server/store/cqldb/config.go
- m2m-oauth-server/store/cqldb/store.go
- m2m-oauth-server/store/cqldb/tokens.go
- m2m-oauth-server/store/mongodb/config.go
- m2m-oauth-server/store/mongodb/tokens.go
- m2m-oauth-server/store/mongodb/tokens_test.go
- m2m-oauth-server/test/service.go
- m2m-oauth-server/test/test.go
- m2m-oauth-server/uri/uri.go
- pkg/mongodb/marshal_test.go
- pkg/security/jwt/claims.go
- pkg/security/jwt/validator/validator.go
- pkg/security/openid/config.go
- pkg/time/unixnano.go
- snippet-service/pb/appliedConfiguration.go
- snippet-service/service/http/service.go
- snippet-service/store/appliedConfiguration.go
- test/config/config.go
Additional context used
LanguageTool
m2m-oauth-server/pb/README.md
[duplication] ~12-~12: Possible typo: you repeated a word
Context: ...oauthserver-pb-GetTokensRequest) - Token - [Token.BlackListed](#m2moauthserver-pb-Token-B...(ENGLISH_WORD_REPEAT_RULE)
Markdownlint
m2m-oauth-server/pb/README.md
7-7: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
8-8: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
9-9: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
10-10: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
11-11: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
13-13: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
15-15: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
135-135: null
Link fragments should be valid(MD051, link-fragments)
Additional comments not posted (41)
m2m-oauth-server/store/store.go (6)
1-10: LGTM!The package and import statements are appropriate for the functionalities provided in the file.
12-15: LGTM!The
Iteratorinterface is well-defined and follows Go's idiomatic patterns for iterators.
17-20: LGTM!The type definitions are clear and useful for processing tokens.
22-28: LGTM!The error variables are well-defined and cover relevant error scenarios.
30-44: LGTM!The
MongoIteratorstruct and methods are well-implemented and follow MongoDB's cursor handling patterns.
46-58: LGTM!The
Storeinterface is comprehensive and covers essential token management functionalities.m2m-oauth-server/store/mongodb/store.go (6)
1-16: LGTM!The package and import statements are appropriate for the functionalities provided in the file.
18-20: LGTM!The struct definition is appropriate for extending the functionalities of
pkgMongo.Store.
22-31: LGTM!The constants and index model are well-defined and necessary for the MongoDB operations.
33-50: LGTM!The
Newfunction is well-implemented and handles errors appropriately. It ensures proper initialization and cleanup.
52-60: LGTM!The
clearDatabasesmethod is well-implemented and ensures proper error handling.
62-64: LGTM!The
Closemethod is straightforward and ensures proper resource cleanup.m2m-oauth-server/service/config.go (5)
Line range hint
1-11:
LGTM!The package and import statements are appropriate for the functionalities provided in the file.
17-27: LGTM!The changes to
ClientsConfigtype and validation method are appropriate and ensure proper validation of theStorageconfiguration.
33-37: LGTM!The changes to
Configtype and validation method are appropriate and ensure proper validation of theOAuthSignerconfiguration.Also applies to: 39-50
61-71: LGTM!The changes to
APIsConfigtype and validation method are appropriate and ensure proper validation of theGRPCconfiguration.
76-82: LGTM!The changes to
HTTPConfigtype and validation method are appropriate and ensure proper validation of theAddrconfiguration.m2m-oauth-server/service/http/postToken.go (2)
29-35: Ensure proper handling of emptyAudiencefield.The
Audiencefield is split into a slice of strings. Ensure that empty strings are handled correctly.Verification successful
Ensure proper handling of empty
Audiencefield.The
Audiencefield is split into a slice of strings. The current implementation correctly handles empty strings by setting theAudiencefield to an empty slice, preventing potential nil pointer issues.
postToken.go:
createTokenRequest.Audience = strings.Split(audience, " ")Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `Audience` field is correctly handled when empty or malformed. # Test: Search for the function usage. Expect: Proper handling of empty or malformed `Audience` field. rg --type go -A 5 $'postFormToCreateTokenRequest'Length of output: 1163
Script:
#!/bin/bash # Description: Verify the handling of `createTokenRequest.Audience` after being set by `postFormToCreateTokenRequest`. # Test: Search for the usage and validation of `createTokenRequest.Audience` in the codebase. rg --type go 'createTokenRequest.Audience' -A 5Length of output: 906
56-59: Ensure proper handling of emptyAudiencefield.The
Audiencefield is split into a slice of strings. Ensure that empty strings are handled correctly.m2m-oauth-server/service/http/postToken_test.go (7)
47-53: Test token creation with JSON data.Ensure that the test case correctly validates the creation of a token using JSON data.
61-68: Test token creation with form data.Ensure that the test case correctly validates the creation of a token using form data.
Line range hint
71-83:
Test token creation with JWT.Ensure that the test case correctly validates the creation of a token using a JWT.
86-99: Test token creation with expiration using JWT.Ensure that the test case correctly validates the creation of a token with an expiration time using a JWT.
106-109: Test invalid client ID.Ensure that the test case correctly validates the behavior when an invalid client ID is used.
113-117: Test invalid JWT.Ensure that the test case correctly validates the behavior when an invalid JWT is used.
124-134: Test invalid expiration time.Ensure that the test case correctly validates the behavior when an invalid expiration time is used.
pkg/mongodb/marshal.go (7)
38-58: Ensure proper handling of slices.The function handles slices in the JSON data structure. Ensure that all edge cases, such as missing or malformed data, are correctly handled.
60-94: Ensure proper handling of path segments.The function finds parent elements based on path segments. Ensure that all edge cases, such as missing or malformed data, are correctly handled.
98-107: Ensure proper handling of path splitting.The function splits a path into segments. Ensure that all edge cases, such as malformed paths, are correctly handled.
109-128: Ensure proper handling of map values.The function sets a value in a map based on the provided path segment. Ensure that all edge cases, such as missing or malformed data, are correctly handled.
149-165: Ensure proper handling of slice values.The function sets a value in a slice based on the provided path segment. Ensure that all edge cases, such as missing or malformed data, are correctly handled.
175-217: Ensure proper handling of path conversion.The function converts a path in the JSON data structure to the appropriate value. Ensure that all edge cases, such as missing or malformed data, are correctly handled.
27-36: Ensure proper handling of missing paths.The function includes an option to permit missing paths. Ensure that this option is correctly handled and tested.
Verification successful
Ensure proper handling of missing paths.
The function
ConvertStringValueToInt64includes an option to permit missing paths. The provided context indicates that this option is correctly handled and tested, as evidenced by the function's usage in multiple locations and the presence of test cases.
Function Usage:
snippet-service/pb/appliedConfiguration.gom2m-oauth-server/pb/token.goTest Cases:
pkg/mongodb/marshal_test.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `permitMissingPaths` option is correctly handled and tested. # Test: Search for the function usage. Expect: Proper handling and testing of `permitMissingPaths` option. rg --type go -A 5 $'ConvertStringValueToInt64'Length of output: 4723
m2m-oauth-server/pb/README.md (5)
6-17: Fix unordered list indentation.The unordered list indentation should be corrected to follow Markdown best practices.
- - [m2m-oauth-server/pb/service.proto](#m2m-oauth-server_pb_service-proto) - - [BlacklistTokensRequest](#m2moauthserver-pb-BlacklistTokensRequest) - - [BlacklistTokensResponse](#m2moauthserver-pb-BlacklistTokensResponse) - - [CreateTokenRequest](#m2moauthserver-pb-CreateTokenRequest) - - [CreateTokenResponse](#m2moauthserver-pb-CreateTokenResponse) - - [GetTokensRequest](#m2moauthserver-pb-GetTokensRequest) - - [Token](#m2moauthserver-pb-Token) - - [Token.BlackListed](#m2moauthserver-pb-Token-BlackListed) - - [M2MOAuthService](#m2moauthserver-pb-M2MOAuthService) - - [Scalar Value Types](#scalar-value-types) + - [m2m-oauth-server/pb/service.proto](#m2m-oauth-server_pb_service-proto) + - [BlacklistTokensRequest](#m2moauthserver-pb-BlacklistTokensRequest) + - [BlacklistTokensResponse](#m2moauthserver-pb-BlacklistTokensResponse) + - [CreateTokenRequest](#m2moauthserver-pb-CreateTokenRequest) + - [CreateTokenResponse](#m2moauthserver-pb-CreateTokenResponse) + - [GetTokensRequest](#m2moauthserver-pb-GetTokensRequest) + - [Token](#m2moauthserver-pb-Token) + - [Token.BlackListed](#m2moauthserver-pb-Token-BlackListed) + - [M2MOAuthService](#m2moauthserver-pb-M2MOAuthService) + - [Scalar Value Types](#scalar-value-types)Tools
LanguageTool
[duplication] ~12-~12: Possible typo: you repeated a word
Context: ...oauthserver-pb-GetTokensRequest) - Token - [Token.BlackListed](#m2moauthserver-pb-Token-B...(ENGLISH_WORD_REPEAT_RULE)
Markdownlint
7-7: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
8-8: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
9-9: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
10-10: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
11-11: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
13-13: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
15-15: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
126-126: Fix table formatting for theidfield.The table for the
idfield is missing a trailing pipe.- | id | [string](#string) | | Token ID / jti + | id | [string](#string) | | Token ID / jti |
166-176: LGTM!The service definitions are clear and follow the gRPC conventions.
181-199: LGTM!The scalar value types are well-documented and follow the protocol buffer conventions.
12-12: Fix possible typo: repeated word.There is a possible typo: the word "Token" is repeated.
- [Token](#m2moauthserver-pb-Token) - [Token.BlackListed](#m2moauthserver-pb-Token-BlackListed) + [Token](#m2moauthserver-pb-Token) - [Token.BlackListed](#m2moauthserver-pb-Token-BlackListed)Likely invalid or redundant comment.
Tools
LanguageTool
[duplication] ~12-~12: Possible typo: you repeated a word
Context: ...oauthserver-pb-GetTokensRequest) - Token - [Token.BlackListed](#m2moauthserver-pb-Token-B...(ENGLISH_WORD_REPEAT_RULE)
Markdownlint
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
m2m-oauth-server/service/http/getTokens_test.go (3)
27-53: Avoid usingdeferinside a loop.Using
deferinside a loop can lead to resource leaks. Consider closing the response body explicitly inside the loop.- defer func() { - _ = resp.Body.Close() - }() + _ = resp.Body.Close()
57-72: Avoid usingdeferinside a loop.Using
deferinside a loop can lead to resource leaks. Consider closing the response body explicitly inside the loop.- defer func() { - _ = resp.Body.Close() - }() + _ = resp.Body.Close()
74-278: Avoid usingdeferinside a loop.Using
deferinside a loop can lead to resource leaks. Consider closing the response body explicitly inside the loop.- defer func() { - _ = resp.Body.Close() - }() + _ = resp.Body.Close()
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (4)
pkg/mongodb/marshal.go (4)
72-84: Unit tests foriterateOverSlicefunction are missing.The
iterateOverSlicefunction is not directly tested. Please add unit tests to ensure its correctness and robustness.
- Add unit tests for the
iterateOverSlicefunction.Analysis chain
LGTM! New function for slice traversal.
The function handles slice traversal and missing paths correctly. Ensure that unit tests cover this new function.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if unit tests cover the `iterateOverSlice` function. # Test: Search for test cases. Expect: Test cases for `iterateOverSlice`. rg --type go -A 5 'func TestIterateOverSlice'Length of output: 47
Script:
#!/bin/bash # Description: Verify if the `iterateOverSlice` function is referenced in any test files. # Search for references to `iterateOverSlice` in Go test files. rg --type go 'iterateOverSlice'Length of output: 231
133-152: Unit tests for thesetMapfunction are missing.The
setMapfunction is not currently covered by any unit tests. Please add unit tests to ensure this function is properly tested.
pkg/mongodb/marshal.go:setMapfunctionAnalysis chain
LGTM! New function for setting values in a map.
The function handles value retrieval and conversion correctly. Ensure that unit tests cover this new function.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if unit tests cover the `setMap` function. # Test: Search for test cases. Expect: Test cases for `setMap`. rg --type go -A 5 'func TestSetMap'Length of output: 37
Script:
#!/bin/bash # Description: Verify if `setMap` is referenced in any test files. # Search for references to `setMap` in Go test files. rg --type go 'setMap' --glob '*_test.go'Length of output: 40
Script:
#!/bin/bash # Description: Verify if `setMap` is referenced in the entire codebase. # Search for references to `setMap` in the entire codebase. rg --type go 'setMap'Length of output: 260
LGTM! Improved flexibility and error handling.
The changes to the function signature enhance flexibility and error handling. The unit tests in
pkg/mongodb/marshal_test.gocover various scenarios, ensuring the new functionality is well-tested.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if unit tests cover the `ConvertStringValueToInt64` function. # Test: Search for test cases. Expect: Test cases for `ConvertStringValueToInt64`. rg --type go -A 5 'func TestConvertStringValueToInt64'Length of output: 387
Script:
#!/bin/bash # Description: Extract and review test cases for `ConvertStringValueToInt64` to confirm coverage of new functionality. # Extract the entire `TestConvertStringValueToInt64` function for review. rg --type go -A 50 'func TestConvertStringValueToInt64' pkg/mongodb/marshal_test.goLength of output: 973
38-58: LGTM! New function for handling slices.The function correctly processes slices and uses
multierrorfor error aggregation. Ensure that unit tests cover this new function.
86-118: LGTM! New function for finding parent elements.The function handles maps and slices correctly to find parent elements. Ensure that unit tests cover this new function.
There was a problem hiding this comment.
Actionable comments posted: 7
Outside diff range, codebase verification and nitpick comments (1)
m2m-oauth-server/service/grpc/token_internal_test.go (1)
12-112: Ensure comprehensive test coverage and edge cases.The test function
TestGetExpirationTimecovers various scenarios for calculating token expiration times. However, consider adding edge cases such as:
- Very large expiration times.
- Expiration times close to the Unix epoch.
- Expiration times far in the future beyond typical usage.
Additionally, ensure that the function handles potential time zone differences correctly.
- File:
m2m-oauth-server/service/grpc/token_internal_test.go- Lines: 12-112
Analysis chain
Ensure comprehensive test coverage and edge cases.
The test function
TestGetExpirationTimecovers various scenarios for calculating token expiration times. However, consider adding edge cases such as:
- Very large expiration times.
- Expiration times close to the Unix epoch.
- Expiration times far in the future beyond typical usage.
Additionally, ensure that the function handles potential time zone differences correctly.
Let's extract the entire function definition for
TestGetExpirationTimeto review the existing test cases and determine if additional edge cases are needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the test coverage for edge cases in `TestGetExpirationTime`. # Test: Search for additional test cases. Expect: Comprehensive coverage including edge cases. rg --type go -A 5 $'TestGetExpirationTime'Length of output: 501
Script:
#!/bin/bash # Extract the entire function definition for `TestGetExpirationTime` to review the test cases. ast-grep --lang go --pattern $'func TestGetExpirationTime(t *testing.T) { $$$ }' --jsonLength of output: 6277
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
m2m-oauth-server/pb/service.pb.gois excluded by!**/*.pb.go,!**/*.pb.gom2m-oauth-server/pb/service.swagger.jsonis excluded by!**/*.jsonm2m-oauth-server/swagger.yamlis excluded by!**/*.yaml
Files selected for processing (8)
- m2m-oauth-server/pb/README.md (1 hunks)
- m2m-oauth-server/pb/doc.html (1 hunks)
- m2m-oauth-server/pb/service.proto (1 hunks)
- m2m-oauth-server/service/grpc/token.go (1 hunks)
- m2m-oauth-server/service/grpc/token_internal_test.go (1 hunks)
- m2m-oauth-server/service/http/postToken_test.go (3 hunks)
- m2m-oauth-server/store/mongodb/tokens.go (1 hunks)
- pkg/mongodb/marshal.go (2 hunks)
Files skipped from review due to trivial changes (1)
- m2m-oauth-server/pb/service.proto
Files skipped from review as they are similar to previous changes (3)
- m2m-oauth-server/pb/doc.html
- m2m-oauth-server/service/grpc/token.go
- pkg/mongodb/marshal.go
Additional context used
LanguageTool
m2m-oauth-server/pb/README.md
[duplication] ~12-~12: Possible typo: you repeated a word
Context: ...oauthserver-pb-GetTokensRequest) - Token - [Token.BlackListed](#m2moauthserver-pb-Token-B...(ENGLISH_WORD_REPEAT_RULE)
Markdownlint
m2m-oauth-server/pb/README.md
7-7: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
8-8: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
9-9: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
10-10: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
11-11: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
13-13: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
15-15: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
134-134: null
Link fragments should be valid(MD051, link-fragments)
Additional comments not posted (5)
m2m-oauth-server/service/http/postToken_test.go (1)
Line range hint
19-146:
Ensure comprehensive test coverage and edge cases.The test function
TestPostTokencovers various scenarios for thePostTokenfunctionality. However, consider adding edge cases such as:
- Invalid token formats.
- Tokens with missing required claims.
- Tokens with future issuance dates.
Additionally, ensure that the function handles potential time zone differences correctly.
m2m-oauth-server/pb/README.md (4)
28-37: LGTM!The
BlacklistTokensRequestmessage type is well-defined.
43-52: LGTM!The
BlacklistTokensResponsemessage type is well-defined.
59-74: LGTM!The
CreateTokenRequestmessage type is well-defined.
180-198: LGTM!The
Scalar Value Typessection is well-defined.
jkralik
force-pushed
the
jkralik/feature/m2m-oauth-server-blacklist
branch
from
f88966e to
1a746f6
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (3)
m2m-oauth-server/pb/README.md (1)
6-17: Fix unordered list indentation.The unordered list indentation should be corrected to follow Markdown best practices.
- - [m2m-oauth-server/pb/service.proto](#m2m-oauth-server_pb_service-proto) - - [BlacklistTokensRequest](#m2moauthserver-pb-BlacklistTokensRequest) - - [BlacklistTokensResponse](#m2moauthserver-pb-BlacklistTokensResponse) - - [CreateTokenRequest](#m2moauthserver-pb-CreateTokenRequest) - - [CreateTokenResponse](#m2moauthserver-pb-CreateTokenResponse) - - [GetTokensRequest](#m2moauthserver-pb-GetTokensRequest) - - [Token](#m2moauthserver-pb-Token) - - [Token.BlackListed](#m2moauthserver-pb-Token-BlackListed) - - [M2MOAuthService](#m2moauthserver-pb-M2MOAuthService) - - [Scalar Value Types](#scalar-value-types) + - [m2m-oauth-server/pb/service.proto](#m2m-oauth-server_pb_service-proto) + - [BlacklistTokensRequest](#m2moauthserver-pb-BlacklistTokensRequest) + - [BlacklistTokensResponse](#m2moauthserver-pb-BlacklistTokensResponse) + - [CreateTokenRequest](#m2moauthserver-pb-CreateTokenRequest) + - [CreateTokenResponse](#m2moauthserver-pb-CreateTokenResponse) + - [GetTokensRequest](#m2moauthserver-pb-GetTokensRequest) + - [Token](#m2moauthserver-pb-Token) + - [Token.BlackListed](#m2moauthserver-pb-Token-BlackListed) + - [M2MOAuthService](#m2moauthserver-pb-M2MOAuthService) + - [Scalar Value Types](#scalar-value-types)Tools
LanguageTool
[duplication] ~12-~12: Possible typo: you repeated a word
Context: ...oauthserver-pb-GetTokensRequest) - Token - [Token.BlackListed](#m2moauthserver-pb-Token-B...(ENGLISH_WORD_REPEAT_RULE)
Markdownlint
7-7: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
8-8: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
9-9: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
10-10: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
11-11: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
13-13: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
15-15: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
pkg/mongodb/marshal.go (2)
38-58: Ensure edge case handling and performance testing forhandleSliceand related functions.The functions
handleSlice,findParents, anditerateOverSlicedo not have direct test coverage in thepkg/mongodb/marshal_test.gofile. To ensure robustness, please add unit tests that cover various edge cases and performance scenarios for these functions.
- Add direct unit tests for
handleSlice,findParents, anditerateOverSliceinpkg/mongodb/marshal_test.go.Analysis chain
LGTM! Verify edge case handling and performance.
The function changes are approved. The function is well-structured and handles errors appropriately using
multierror.However, ensure that the function handles all edge cases and maintains performance.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify edge case handling and performance of handleSlice function. # Test: Search for the function usage. Expect: Proper handling of edge cases and performance. rg --type go -A 5 $'handleSlice'Length of output: 723
Script:
#!/bin/bash # Extract the context of handleSlice usage within pkg/mongodb/marshal.go and any related functions. # Search for all functions in the file to understand the surrounding context ast-grep --lang go --pattern $'func $_($_) $_ { $$$ }' pkg/mongodb/marshal.go # Extract the full implementation of handleSlice and related functions rg --type go -A 20 'func handleSlice' pkg/mongodb/marshal.go rg --type go -A 20 'func findParents' pkg/mongodb/marshal.go rg --type go -A 20 'func iterateOverSlice' pkg/mongodb/marshal.goLength of output: 2306
Script:
#!/bin/bash # Search for test cases related to handleSlice, findParents, and iterateOverSlice to verify edge case handling and performance. # Find test files fd --type f --name '*_test.go' # Search for test cases involving handleSlice rg --type go 'handleSlice' $(fd --type f --name '*_test.go') # Search for test cases involving findParents rg --type go 'findParents' $(fd --type f --name '*_test.go') # Search for test cases involving iterateOverSlice rg --type go 'iterateOverSlice' $(fd --type f --name '*_test.go')Length of output: 1937
Script:
#!/bin/bash # Search for test cases related to handleSlice, findParents, and iterateOverSlice to verify edge case handling and performance. # Find test files with the correct flag fd -e go '_test.go' # Search for test cases involving handleSlice rg --type go 'handleSlice' $(fd -e go '_test.go') # Search for test cases involving findParents rg --type go 'findParents' $(fd -e go '_test.go') # Search for test cases involving iterateOverSlice rg --type go 'iterateOverSlice' $(fd -e go '_test.go')Length of output: 56860
Script:
#!/bin/bash # List all test functions in pkg/mongodb/marshal_test.go and search for mentions of handleSlice, findParents, and iterateOverSlice. # List all test functions ast-grep --lang go --pattern $'func Test$_($_) $_ { $$$ }' pkg/mongodb/marshal_test.go # Search for mentions of handleSlice, findParents, and iterateOverSlice within the test file rg --type go 'handleSlice' pkg/mongodb/marshal_test.go rg --type go 'findParents' pkg/mongodb/marshal_test.go rg --type go 'iterateOverSlice' pkg/mongodb/marshal_test.goLength of output: 257
72-84: Add test cases foriterateOverSliceto ensure edge case handling and performance.The function
iterateOverSliceis well-structured and handles errors appropriately. However, there are no existing test cases covering this function. To ensure it handles all edge cases and performs well, please add test cases that cover scenarios such as:
- Negative indices
- Out-of-range indices
- Non-integer parts
- Behavior when
permitMissingPathsis true or falseThis will help verify the robustness and performance of the function.
Analysis chain
LGTM! Verify edge case handling and performance.
The function changes are approved. The function is well-structured and handles errors appropriately.
However, ensure that the function handles all edge cases and maintains performance.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify edge case handling and performance of iterateOverSlice function. # Test: Search for the function usage. Expect: Proper handling of edge cases and performance. rg --type go -A 5 $'iterateOverSlice'Length of output: 757
Script:
#!/bin/bash # Description: Search for test cases related to the iterateOverSlice function to verify edge case handling. # Find all test files fd --type f --name '*_test.go' > test_files.txt # Search for test cases that involve iterateOverSlice while IFS= read -r file; do echo "Searching in $file" rg --type go 'iterateOverSlice' "$file" done < test_files.txtLength of output: 305
Script:
#!/bin/bash # Description: Search for test cases related to the iterateOverSlice function to verify edge case handling. # Find all test files fd '\.go$' > test_files.txt # Search for test cases that involve iterateOverSlice while IFS= read -r file; do echo "Searching in $file" rg --type go 'iterateOverSlice' "$file" done < test_files.txtLength of output: 201247
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (21)
charts/plgd-hub/templates/certificate-authority/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/grpc-ingress.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/grpc-service.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/http-ingress.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/http-service.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/service-crt.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/m2m-oauth-server/service.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/snippet-service/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/templates/snippet-service/deployment.yamlis excluded by!**/*.yamlcharts/plgd-hub/values.yamlis excluded by!**/*.yamlgrpc-gateway/pb/hubConfiguration.pb.gois excluded by!**/*.pb.go,!**/*.pb.gom2m-oauth-server/config.yamlis excluded by!**/*.yamlm2m-oauth-server/pb/service.pb.gois excluded by!**/*.pb.go,!**/*.pb.gom2m-oauth-server/pb/service.pb.gw.gois excluded by!**/*.pb.gw.go,!**/*.pb.gw.gom2m-oauth-server/pb/service.swagger.jsonis excluded by!**/*.jsonm2m-oauth-server/pb/service_grpc.pb.gois excluded by!**/*.pb.go,!**/*.pb.gom2m-oauth-server/swagger.yamlis excluded by!**/*.yamlsnippet-service/pb/service.pb.gois excluded by!**/*.pb.go,!**/*.pb.gotest/helm/mock.plgd.cloud.yamlis excluded by!**/*.yaml
Files selected for processing (55)
- Makefile (1 hunks)
- charts/plgd-hub/templates/m2m-oauth-server/_helpers.tpl (3 hunks)
- m2m-oauth-server/Makefile (3 hunks)
- m2m-oauth-server/oauthSigner/config.go (1 hunks)
- m2m-oauth-server/oauthSigner/loadKeys.go (1 hunks)
- m2m-oauth-server/oauthSigner/oauthSigner.go (1 hunks)
- m2m-oauth-server/pb/README.md (1 hunks)
- m2m-oauth-server/pb/doc.html (1 hunks)
- m2m-oauth-server/pb/service.proto (1 hunks)
- m2m-oauth-server/pb/tags.go (1 hunks)
- m2m-oauth-server/pb/token.go (1 hunks)
- m2m-oauth-server/service/config.go (2 hunks)
- m2m-oauth-server/service/expiredUpdatesChecker.go (1 hunks)
- m2m-oauth-server/service/grpc/config.go (1 hunks)
- m2m-oauth-server/service/grpc/server.go (1 hunks)
- m2m-oauth-server/service/grpc/service.go (1 hunks)
- m2m-oauth-server/service/grpc/token.go (1 hunks)
- m2m-oauth-server/service/grpc/token_internal_test.go (1 hunks)
- m2m-oauth-server/service/http/config.go (1 hunks)
- m2m-oauth-server/service/http/createToken_test.go (1 hunks)
- m2m-oauth-server/service/http/getJWKs.go (2 hunks)
- m2m-oauth-server/service/http/getJWKs_test.go (1 hunks)
- m2m-oauth-server/service/http/getOpenIDConfiguration.go (2 hunks)
- m2m-oauth-server/service/http/getOpenIDConfiguration_test.go (1 hunks)
- m2m-oauth-server/service/http/getTokens_test.go (1 hunks)
- m2m-oauth-server/service/http/jsonWriter.go (1 hunks)
- m2m-oauth-server/service/http/postToken.go (1 hunks)
- m2m-oauth-server/service/http/postToken_test.go (3 hunks)
- m2m-oauth-server/service/http/requestHandler.go (1 hunks)
- m2m-oauth-server/service/http/service.go (1 hunks)
- m2m-oauth-server/service/service.go (1 hunks)
- m2m-oauth-server/service/service_test.go (1 hunks)
- m2m-oauth-server/store/config/config.go (1 hunks)
- m2m-oauth-server/store/config/config_test.go (1 hunks)
- m2m-oauth-server/store/cqldb/config.go (1 hunks)
- m2m-oauth-server/store/cqldb/store.go (1 hunks)
- m2m-oauth-server/store/cqldb/tokens.go (1 hunks)
- m2m-oauth-server/store/mongodb/config.go (1 hunks)
- m2m-oauth-server/store/mongodb/store.go (1 hunks)
- m2m-oauth-server/store/mongodb/tokens.go (1 hunks)
- m2m-oauth-server/store/mongodb/tokens_test.go (1 hunks)
- m2m-oauth-server/store/store.go (1 hunks)
- m2m-oauth-server/test/service.go (1 hunks)
- m2m-oauth-server/test/test.go (7 hunks)
- m2m-oauth-server/uri/uri.go (1 hunks)
- pkg/mongodb/marshal.go (2 hunks)
- pkg/mongodb/marshal_test.go (1 hunks)
- pkg/security/jwt/claims.go (1 hunks)
- pkg/security/jwt/validator/validator.go (3 hunks)
- pkg/security/openid/config.go (1 hunks)
- pkg/time/unixnano.go (1 hunks)
- snippet-service/pb/appliedConfiguration.go (2 hunks)
- snippet-service/service/http/service.go (2 hunks)
- snippet-service/store/appliedConfiguration.go (1 hunks)
- test/config/config.go (1 hunks)
Files skipped from review due to trivial changes (10)
- m2m-oauth-server/pb/tags.go
- m2m-oauth-server/service/grpc/config.go
- m2m-oauth-server/service/grpc/server.go
- m2m-oauth-server/service/http/config.go
- m2m-oauth-server/service/http/requestHandler.go
- m2m-oauth-server/service/service_test.go
- m2m-oauth-server/store/cqldb/store.go
- m2m-oauth-server/store/cqldb/tokens.go
- m2m-oauth-server/store/mongodb/config.go
- snippet-service/service/http/service.go
Files skipped from review as they are similar to previous changes (38)
- Makefile
- charts/plgd-hub/templates/m2m-oauth-server/_helpers.tpl
- m2m-oauth-server/Makefile
- m2m-oauth-server/oauthSigner/config.go
- m2m-oauth-server/oauthSigner/loadKeys.go
- m2m-oauth-server/pb/doc.html
- m2m-oauth-server/pb/service.proto
- m2m-oauth-server/pb/token.go
- m2m-oauth-server/service/config.go
- m2m-oauth-server/service/expiredUpdatesChecker.go
- m2m-oauth-server/service/grpc/service.go
- m2m-oauth-server/service/grpc/token.go
- m2m-oauth-server/service/grpc/token_internal_test.go
- m2m-oauth-server/service/http/createToken_test.go
- m2m-oauth-server/service/http/getJWKs.go
- m2m-oauth-server/service/http/getJWKs_test.go
- m2m-oauth-server/service/http/getOpenIDConfiguration.go
- m2m-oauth-server/service/http/getOpenIDConfiguration_test.go
- m2m-oauth-server/service/http/getTokens_test.go
- m2m-oauth-server/service/http/jsonWriter.go
- m2m-oauth-server/service/http/service.go
- m2m-oauth-server/service/service.go
- m2m-oauth-server/store/config/config_test.go
- m2m-oauth-server/store/cqldb/config.go
- m2m-oauth-server/store/mongodb/store.go
- m2m-oauth-server/store/mongodb/tokens_test.go
- m2m-oauth-server/store/store.go
- m2m-oauth-server/test/service.go
- m2m-oauth-server/test/test.go
- m2m-oauth-server/uri/uri.go
- pkg/mongodb/marshal_test.go
- pkg/security/jwt/claims.go
- pkg/security/jwt/validator/validator.go
- pkg/security/openid/config.go
- pkg/time/unixnano.go
- snippet-service/pb/appliedConfiguration.go
- snippet-service/store/appliedConfiguration.go
- test/config/config.go
Additional context used
LanguageTool
m2m-oauth-server/pb/README.md
[duplication] ~12-~12: Possible typo: you repeated a word
Context: ...oauthserver-pb-GetTokensRequest) - Token - [Token.BlackListed](#m2moauthserver-pb-Token-B...(ENGLISH_WORD_REPEAT_RULE)
Markdownlint
m2m-oauth-server/pb/README.md
7-7: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
8-8: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
9-9: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
10-10: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
11-11: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
13-13: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
15-15: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
134-134: null
Link fragments should be valid(MD051, link-fragments)
Additional comments not posted (45)
m2m-oauth-server/store/config/config.go (3)
20-23: Ensure comprehensive error handling for configuration validation.The validation of the configuration is crucial. Ensure that all potential errors are handled and logged appropriately.
31-35: Improve resource management for the cron job scheduler.The deferred shutdown of the scheduler is good practice. Ensure that the scheduler is properly managed and resources are released.
36-42: Handle potential errors in cron job setup.Ensure that potential errors in setting up the cron job are handled and logged.
m2m-oauth-server/oauthSigner/oauthSigner.go (13)
19-21: LGTM!The helper function is correctly implemented.
31-35: Handle potential errors in private key loading.Ensure that potential errors in loading the private key are handled and logged appropriately.
36-39: Handle potential errors in JWK creation.Ensure that potential errors in creating the JWK are handled and logged appropriately.
47-54: Ensure proper resource management for validators.The deferred execution of the closer is good practice. Ensure that all resources are properly managed and released.
64-67: LGTM!The getter function is correctly implemented.
69-76: Handle potential errors in header setting.Ensure that potential errors in setting the headers are handled and logged appropriately.
78-81: Handle potential errors in data signing.Ensure that potential errors in signing the data are handled and logged appropriately.
85-87: LGTM!The getter function is correctly implemented.
89-93: Handle potential errors in token encoding.Ensure that potential errors in encoding the token are handled and logged appropriately.
94-95: Handle potential errors in token signing.Ensure that potential errors in signing the token are handled and logged appropriately.
97-99: LGTM!The function is correctly implemented.
101-103: LGTM!The getter function is correctly implemented.
105-107: LGTM!The getter function is correctly implemented.
m2m-oauth-server/service/http/postToken.go (6)
29-50: Handle potential errors in expiration parsing.Ensure that potential errors in parsing the expiration value are handled and logged appropriately.
53-69: LGTM!The function correctly maps JSON data to the request object.
71-79: Handle potential errors in form parsing.Ensure that potential errors in parsing the form data are handled and logged appropriately.
83-87: Handle potential errors in JSON parsing.Ensure that potential errors in parsing the JSON data are handled and logged appropriately.
109-111: Handle potential errors in response writing.Ensure that potential errors in writing the response are handled and logged appropriately.
109-111: Handle potential errors in response writing.Ensure that potential errors in writing the response are handled and logged appropriately.
m2m-oauth-server/store/mongodb/tokens.go (6)
17-35: Ensure comprehensive validation and error handling.The
CreateTokenfunction correctly assigns default values and validates the token. However, consider adding more comprehensive validation, such as:
- Checking for duplicate token IDs before insertion.
- Ensuring all required fields are present and valid.
- Handling potential MongoDB errors more gracefully.
Additionally, consider logging errors for better traceability.
+ // Check for duplicate token IDs + count, err := s.Store.Collection(tokensCol).CountDocuments(ctx, bson.M{"_id": token.Id}) + if err != nil { + return nil, err + } + if count > 0 { + return nil, store.ErrDuplicateTokenID + } // Insert the token _, err = s.Store.Collection(tokensCol).InsertOne(ctx, token) if err != nil { // Log the error for better traceability log.Errorf("Failed to insert token: %v", err) return nil, err }
38-70: Optimize filter generation for efficiency.The
toFilterfunction correctly generates a MongoDB filter based on the provided criteria. However, consider optimizing the filter generation for efficiency:
- Use indexed fields for filtering whenever possible.
- Avoid unnecessary conditions that may slow down the query.
Additionally, ensure that the generated filter is thoroughly tested for all possible scenarios.
if len(req.GetIdFilter()) > 0 { filter = append(filter, bson.E{Key: "_id", Value: bson.M{mongodb.In: req.GetIdFilter()}}) } else { setIdOwnerHint = false } if owner != "" { filter = append(filter, bson.E{Key: pb.OwnerKey, Value: owner}) } else { setIdOwnerHint = false } + // Ensure indexed fields are used for filtering + if len(req.GetIdFilter()) > 0 && owner != "" { + filter = append(filter, bson.E{Key: "_id", Value: bson.M{mongodb.In: req.GetIdFilter()}}, bson.E{Key: pb.OwnerKey, Value: owner}) + setIdOwnerHint = true + } else { + setIdOwnerHint = false + }
72-91: Ensure robust error handling and efficient cursor processing.The
processCursorfunction processes a MongoDB cursor and applies a given process function to each document. Ensure robust error handling and efficient cursor processing:
- Handle potential cursor errors more gracefully.
- Optimize the processing loop for better performance.
Additionally, consider logging errors for better traceability.
for { var stored T if !iter.Next(ctx, &stored) { break } err := process(&stored) if err != nil { errors = multierror.Append(errors, err) + // Log the error for better traceability + log.Errorf("Failed to process document: %v", err) break } } errors = multierror.Append(errors, iter.Err()) errClose := cr.Close(ctx) errors = multierror.Append(errors, errClose) + // Log the cursor close error for better traceability + if errClose != nil { + log.Errorf("Failed to close cursor: %v", errClose) + } return errors.ErrorOrNil()
94-108: Ensure efficient token retrieval and robust error handling.The
GetTokensfunction retrieves tokens from the MongoDB store based on the provided criteria. Ensure efficient token retrieval and robust error handling:
- Use indexed fields for filtering whenever possible.
- Handle potential MongoDB errors more gracefully.
Additionally, consider logging errors for better traceability.
filter, hint := toFilter(owner, req) opts := options.Find() if hint != nil { opts.SetHint(hint) } cur, err := s.Store.Collection(tokensCol).Find(ctx, filter, opts) if err != nil { // Log the error for better traceability log.Errorf("Failed to retrieve tokens: %v", err) return err } return processCursor(ctx, cur, process)
110-117: Ensure efficient token deletion and robust error handling.The
DeleteTokensfunction deletes tokens from the MongoDB store based on the provided criteria. Ensure efficient token deletion and robust error handling:
- Use indexed fields for filtering whenever possible.
- Handle potential MongoDB errors more gracefully.
Additionally, consider logging errors for better traceability.
deleteFilter := bson.D{ {Key: pb.ExpirationKey, Value: bson.M{"$lt": now.Unix()}}, {Key: pb.ExpirationKey, Value: bson.M{"$gt": int64(0)}}, {Key: pb.BlackListedFlagKey, Value: true}, } _, err := s.Store.Collection(tokensCol).DeleteMany(ctx, deleteFilter) if err != nil { // Log the error for better traceability log.Errorf("Failed to delete tokens: %v", err) } return err
120-167: Ensure efficient token blacklisting and robust error handling.The
BlacklistTokensfunction blacklists tokens based on the provided criteria. Ensure efficient token blacklisting and robust error handling:
- Use indexed fields for filtering whenever possible.
- Handle potential MongoDB errors more gracefully.
Additionally, consider logging errors for better traceability.
filter := bson.D{ {Key: pb.OwnerKey, Value: owner}, { Key: mongodb.Or, Value: bson.A{ bson.M{ pb.ExpirationKey: bson.M{"$gte": time.Now().Unix()}, }, bson.M{ pb.ExpirationKey: bson.M{mongodb.Exists: false}, }, bson.M{ pb.ExpirationKey: int64(0), }, }, }, { Key: mongodb.Or, Value: bson.A{ bson.M{pb.BlackListedFlagKey: false}, bson.M{pb.BlackListedFlagKey: bson.M{mongodb.Exists: false}}, }, }, } if len(req.GetIdFilter()) > 0 { filter = append(filter, bson.E{Key: "_id", Value: bson.M{mongodb.In: req.GetIdFilter()}}) } blacklisted := pb.Token_BlackListed{ Flag: true, Timestamp: time.Now().Unix(), } update := bson.D{ { Key: mongodb.Set, Value: bson.M{ pb.BlackListedKey: &blacklisted, }, }, } ret, err := s.Store.Collection(tokensCol).UpdateMany(ctx, filter, update) if err != nil { // Log the error for better traceability log.Errorf("Failed to blacklist tokens: %v", err) return nil, err } return &pb.BlacklistTokensResponse{ Count: ret.MatchedCount, }, nilm2m-oauth-server/service/http/postToken_test.go (12)
1-1: Package name change is appropriate.The package name has been changed from
service_testtohttp_testto better reflect the context of the tests.
19-19: Function renaming is appropriate.The function name has been changed from
TestGetTokentoTestPostTokento accurately reflect the functionality being tested.
11-11: Updated import statement is appropriate.The import statement for
github.com/plgd-dev/hub/v2/m2m-oauth-server/servicehas been updated togithub.com/plgd-dev/hub/v2/m2m-oauth-server/oauthSignerto reflect the new package structure.
47-47: Updated usage ofGrantTypeClientCredentialsis appropriate.The usage of
service.GrantTypeClientCredentialshas been replaced withoauthsigner.GrantTypeClientCredentialsto reflect the new import statement.
51-53: New test case for validating token owner is appropriate.The
wantstruct has been modified to include anownerfield, and the test case validates the token owner correctly.
66-68: New test case for validating token owner in postForm is appropriate.The
wantstruct has been modified to include anownerfield, and the test case validates the token owner correctly when using postForm.
Line range hint
71-82:
New test case for validating token owner with JWT is appropriate.The
wantstruct has been modified to include anownerfield, and the test case validates the token owner and the existence of original token claims correctly when using JWT.
86-100: New test case for validating token expiration with JWT is appropriate.The test case validates the token owner and the existence of original token claims correctly when using JWT with an expiration time.
102-112: New test case for handling over time expiration with JWT is appropriate.The test case correctly handles the scenario where the token expiration time exceeds the allowed limit, resulting in an unauthorized status.
115-123: New test case for handling invalid client is appropriate.The test case correctly handles the scenario where an invalid client is used, resulting in an unauthorized status.
125-133: New test case for handling invalid JWT is appropriate.The test case correctly handles the scenario where an invalid JWT is used, resulting in an unauthorized status.
136-146: New test case for handling invalid expiration is appropriate.The test case correctly handles the scenario where the token expiration time is invalid, resulting in an unauthorized status.
m2m-oauth-server/pb/README.md (1)
28-37: Fix table formatting for theid_filterfield.The table for the
id_filterfield is missing a trailing pipe.- | id_filter </blockquote></details> <details> <summary>pkg/mongodb/marshal.go (4)</summary><blockquote> `27-36`: **LGTM! Verify edge case handling and performance.** The function changes are approved. The function is well-documented and handles errors appropriately. However, ensure that the function handles all edge cases and maintains performance. --- `60-70`: **LGTM! Verify edge case handling and performance.** The function changes are approved. The function is well-structured and handles errors appropriately. However, ensure that the function handles all edge cases and maintains performance. <details> <summary>Verification successful</summary> --- <!-- [monologue] [task review_file_1/1 function_3/6] Continue reviewing the `iterateOverMap` function. [observations] The new context shows the `iterateOverMap` function in relation to other functions such as `iterateOverSlice`, `nextCurrent`, and `findParents`. These functions collectively handle different types of data structures and path segments, indicating a comprehensive approach to nested data traversal. [analysis] The `nextCurrent` function uses `iterateOverMap` to handle `map[string]any` types, and it also calls `iterateOverSlice` for `[]any` types. This suggests that the function is part of a larger mechanism to traverse complex nested structures. The error handling and the conditions for `permitMissingPaths` seem consistent and well-integrated. [actions] Based on the gathered context, it is evident that edge cases and performance considerations are addressed through the combined logic of these functions. The function `iterateOverMap` appears to be well-implemented and integrated within the larger context. --> **LGTM! Verify edge case handling and performance.** The function changes are approved. The function is well-structured and handles errors appropriately. The context shows that edge cases and performance considerations are addressed through the combined logic of related functions. </details> <details> <summary>Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Verify edge case handling and performance of iterateOverMap function. # Test: Search for the function usage. Expect: Proper handling of edge cases and performance. rg --type go -A 5 $'iterateOverMap'Length of output: 728
Script:
#!/bin/bash # Description: Retrieve more context around the usage of iterateOverMap function in pkg/mongodb/marshal.go # Get more lines around the usage of iterateOverMap to understand edge case handling and performance considerations rg --type go -A 20 -B 10 $'iterateOverMap' pkg/mongodb/marshal.goLength of output: 1622
113-130: LGTM! Verify edge case handling and performance.The function changes are approved. The function is well-structured and handles errors appropriately.
However, ensure that the function handles all edge cases and maintains performance.
Verification successful
LGTM! Verify edge case handling and performance.
The function changes are approved. The function is well-structured and handles errors appropriately.
- The
findParentsfunction, along with its helper functionsnextCurrent,iterateOverMap,handleSlice, anditerateOverSlice, handles various edge cases and maintains performance efficiently.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify edge case handling and performance of findParents function. # Test: Search for the function usage. Expect: Proper handling of edge cases and performance. rg --type go -A 5 $'findParents'Length of output: 1106
Script:
#!/bin/bash # Description: Locate and analyze the implementation of the nextCurrent function. # Search for the implementation of nextCurrent function in the codebase rg --type go -A 10 'func nextCurrent'Length of output: 627
Script:
#!/bin/bash # Description: Retrieve the full implementation of nextCurrent and iterateOverMap functions. # Search for the complete implementation of nextCurrent function rg --type go -A 20 'func nextCurrent' # Search for the implementation of iterateOverMap function rg --type go -A 10 'func iterateOverMap'Length of output: 1788
Script:
#!/bin/bash # Description: Retrieve the full implementation of handleSlice and iterateOverSlice functions. # Search for the implementation of handleSlice function rg --type go -A 10 'func handleSlice' # Search for the implementation of iterateOverSlice function rg --type go -A 10 'func iterateOverSlice'Length of output: 1284
86-111: LGTM! Verify edge case handling and performance.The function changes are approved. The function is well-structured and handles errors appropriately.
However, ensure that the function handles all edge cases and maintains performance.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
**Token Generation:** Added a form to create new API tokens, where users can specify a custom name and select an expiration date. Included client-side validation to ensure that token names are unique and expiration dates are valid. **Token Listing:** Implemented a new interface to display a list of existing tokens, including their names, creation dates, expiration dates, and status. Added sorting and filtering capabilities to help users quickly find specific tokens. **Token Removal:** Introduced functionality for users to remove tokens directly from the UI. Added confirmation dialogs to prevent accidental deletion of tokens. --------- Co-authored-by: Jozef Kralik <[email protected]> Co-authored-by: Daniel Adam <[email protected]>
|
Overview
This pull request introduces comprehensive functionalities for managing tokens in the M2M OAuth server, enhancing the server's ability to handle token creation, retrieval, blacklisting, and deletion.
Key Features
Token Creation:
Token Retrieval:
Token Blacklisting:
Token Deletion:
Configuration Enhancements:
Protocol Buffers and gRPC:
Documentation:
Web interface:
Benefits
These enhancements make the M2M OAuth server more robust and secure, providing essential features for managing API access tokens in a machine-to-machine context.