search

plgd-dev / hub

Secure and Interoperable Internet of Things
https://plgd.dev
Apache License 2.0
171 stars 17 forks source link

m2moauthserver: fix security vulnerability #1339

Closed jkralik closed 2 months ago

jkralik commented 2 months ago

Summary of Changes

The PR primarily addresses security vulnerabilities in the OAuth server by modifying OAuth client configurations and token handling mechanisms. Key changes include:

  1. certificate-authority/service/grpc/signCertificate_test.go:

    • Updated TestCertificateAuthorityServerSignCSRWithDifferentPublicKeys to include OAuth client setup and token claims insertion.

  2. m2m-oauth-server/service/config.go:

    • Added InsertTokenClaims field to the Client struct.
    • Removed RequireDeviceID and RequireOwner fields from the Client struct.
    • Updated the Validate method accordingly.

  3. m2m-oauth-server/service/token.go:

    • Modified makeAccessToken to incorporate clientCfg.InsertTokenClaims.
    • Refactored tokenRequest struct, including renaming and reordering fields.
    • Updated postToken to use tokenReq.Secret instead of tokenReq.Password.
    • Simplified validation functions by removing checks for DeviceID and Owner.

  4. m2m-oauth-server/service/token_test.go:

    • Simplified TestGetToken by removing certain test cases and focusing on core token retrieval tests.

  5. m2m-oauth-server/test/test.go:

    • Removed RequireDeviceID and RequireOwner fields from OAuth client declarations.
    • Added InsertTokenClaims to ServiceOAuthClient.

  6. m2m-oauth-server/uri/uri.go:

    • Removed several constant declarations and introduced a new constant OriginalTokenClaims.

These changes enhance the security and flexibility of the OAuth server by allowing dynamic insertion of token claims and simplifying the validation logic.

coderabbitai[bot] commented 2 months ago

Walkthrough

The changes introduce updates to OAuth client configurations, focusing on token claims handling and context setup. The Client struct now incorporates an InsertTokenClaims field and removes validations for RequireDeviceID and RequireOwner. Test cases have been streamlined, removing scenarios and simplifying assertions. The alterations also involve modifying functions related to token generation and validation, as well as adjusting constants within the project.

Changes

File Path Change Summary
certificate-authority/service/grpc/signCertificate_test.go Set up OAuth client configurations and inserted token claims before service setup. Modified context token setup.
m2m-oauth-server/service/config.go Added InsertTokenClaims to Client struct. Removed validation for RequireDeviceID and RequireOwner.
m2m-oauth-server/service/token.go Modified functions and structures for token generation and validation. Renamed fields in tokenRequest and updated corresponding handling code.
m2m-oauth-server/service/token_test.go Simplified TestGetToken by removing specific test cases and corresponding configurations. Streamlined assertions.
m2m-oauth-server/test/test.go Removed RequireDeviceID and RequireOwner from declarations. Added InsertTokenClaims to ServiceOAuthClient.
m2m-oauth-server/uri/uri.go Removed UsernameKey, PasswordKey, SubjectKey, DeviceIDKey, OwnerKey constants. Added OriginalTokenClaims constant declaration.

Poem

In the realm of OAuth bright and keen,
Code adjustments float serene.
Token claims now swiftly glide,
Fields once strict, now step aside.
Simpler tests, with claims anew,
A path of tokens just for you.
🎉👾✨

                       (\(\ 
                    ( -   - )
                   c(")__(")

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?

Share - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
Tips ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` 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) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.
sonarcloud[bot] commented 2 months ago

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
76.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

jkralik commented 2 months ago

@coderabbitai generate summary for PR

coderabbitai[bot] commented 2 months ago

@jkralik: Here is the summary for PR #1339:

Summary of Changes

The PR primarily addresses security vulnerabilities in the OAuth server by modifying OAuth client configurations and token handling mechanisms. Key changes include:

  1. certificate-authority/service/grpc/signCertificate_test.go:

    • Updated TestCertificateAuthorityServerSignCSRWithDifferentPublicKeys to include OAuth client setup and token claims insertion.

  2. m2m-oauth-server/service/config.go:

    • Added InsertTokenClaims field to the Client struct.
    • Removed RequireDeviceID and RequireOwner fields from the Client struct.
    • Updated the Validate method accordingly.

  3. m2m-oauth-server/service/token.go:

    • Modified makeAccessToken to incorporate clientCfg.InsertTokenClaims.
    • Refactored tokenRequest struct, including renaming and reordering fields.
    • Updated postToken to use tokenReq.Secret instead of tokenReq.Password.
    • Simplified validation functions by removing checks for DeviceID and Owner.

  4. m2m-oauth-server/service/token_test.go:

    • Simplified TestGetToken by removing certain test cases and focusing on core token retrieval tests.

  5. m2m-oauth-server/test/test.go:

    • Removed RequireDeviceID and RequireOwner fields from OAuth client declarations.
    • Added InsertTokenClaims to ServiceOAuthClient.

  6. m2m-oauth-server/uri/uri.go:

    • Removed several constant declarations and introduced a new constant OriginalTokenClaims.

These changes enhance the security and flexibility of the OAuth server by allowing dynamic insertion of token claims and simplifying the validation logic.

Actions performed Comments resolved.