Skip to content

Add API authentication and panic handling in server webhook process#151

Merged
abbas-dependable-naqvi merged 11 commits intomasterfrom
add_api_authentication
Jun 3, 2025
Merged

Add API authentication and panic handling in server webhook process#151
abbas-dependable-naqvi merged 11 commits intomasterfrom
add_api_authentication

Conversation

@Kshitij-Katiyar
Copy link
Copy Markdown
Contributor

Summary

Add API authentication and panic handling in the server webhook process

Ticket Link

Fixes https://mattermost.atlassian.net/browse/MM-64153
Fixes https://mattermost.atlassian.net/browse/MM-64148

@Kshitij-Katiyar Kshitij-Katiyar self-assigned this May 5, 2025
@Kshitij-Katiyar Kshitij-Katiyar added the 2: Dev Review Requires review by a core committer label May 5, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@Kshitij-Katiyar Kshitij-Katiyar requested a review from Copilot May 5, 2025 12:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request enhances API security and server robustness by adding authentication checks to endpoints and improving error handling in JSON deserialization for Confluence webhooks.

  • Update function signature of ConfluenceServerEventFromJSON to return errors.
  • Add IsAuthenticated flags for multiple endpoints.
  • Introduce a new checkAuth middleware in the controller for secured endpoints.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
server/serializer/confluence_server.go Updated JSON deserialization to return errors in case of failure.
server/save_subscription.go Added IsAuthenticated flag for API authentication.
server/oauth2.go Updated IsAuthenticated flags for OAuth endpoints.
server/get_subscriptions.go Enabled API authentication on subscriptions endpoint.
server/get_subscription.go Enabled API authentication on subscription retrieval endpoint.
server/edit_subscription.go Enabled API authentication on subscription edit endpoint.
server/controller.go Introduced checkAuth middleware and incorporated it into API routing.
server/confluence_server.go Modified webhook handler for proper error handling of JSON payloads.
server/confluence_cloud.go Enabled IsAuthenticated flag on Confluence cloud webhook endpoint.
server/atlassian_connect.go Set IsAuthenticated flag for Atlassian connect endpoint.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements API authentication and panic handling in the server webhook process. Key changes include:

  • Updating header retrieval to use a constant (config.HeaderMattermostUserID) for consistent authentication.
  • Adding the IsAuthenticated flag on various endpoints to enforce authentication where required.
  • Enhancing error handling in the Confluence server webhook payload decoding.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
server/user.go Updated header retrieval for user identification in OAuth endpoints.
server/serializer/confluence_server.go Enhanced JSON decoding to return errors for invalid payloads.
server/save_subscription.go Marked subscription saving endpoint as authenticated.
server/oauth2.go Marked OAuth2 endpoints as authenticated.
server/get_subscriptions.go Updated header retrieval and marked autocomplete subscriptions endpoint as authenticated.
server/get_subscription.go Marked subscription retrieval endpoint as authenticated.
server/edit_subscription.go Marked subscription editing endpoint as authenticated.
server/controller.go Added auth middleware to endpoints; potential duplicate route registration noted.
server/confluence_server.go Improved webhook error handling on JSON unmarshal failure.
server/confluence_cloud.go Explicitly set authentication flag to false.
server/atlassian_connect.go Explicitly set authentication flag to false.

Copy link
Copy Markdown
Member

@wiggin77 wiggin77 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Kshitij-Katiyar
Copy link
Copy Markdown
Contributor Author

@enzowritescode Fixed the comments

@Kshitij-Katiyar
Copy link
Copy Markdown
Contributor Author

fixed the comment @enzowritescode

@abbas-dependable-naqvi abbas-dependable-naqvi merged commit de0a3d4 into master Jun 3, 2025
6 checks passed
@abbas-dependable-naqvi abbas-dependable-naqvi deleted the add_api_authentication branch June 3, 2025 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2: Dev Review Requires review by a core committer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants