Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: oauth access denied handling #3960

Merged
merged 34 commits into from
Oct 18, 2023
Merged

Conversation

sanpj2292
Copy link
Contributor

Description

Resolves INT-367

Linear Ticket

https://linear.app/rudderstack/issue/INT-367/testing-review-and-release

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

Sai Sankeerth added 26 commits September 11, 2023 21:48
Signed-off-by: Sai Sankeerth <sanpj2292@github.com>
Signed-off-by: Sai Sankeerth <sanpj2292@github.com>
- include relevant status-codes
- remove unnecessary printf statements
…ilure & success)

- refactor authStatus toggle handling in deleteUsers
- correction of test-cases in deleteUsers
@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

Attention: 30 lines in your changes are missing coverage. Please review.

Comparison is base (6e0729c) 71.07% compared to head (000c6f9) 71.35%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3960      +/-   ##
==========================================
+ Coverage   71.07%   71.35%   +0.27%     
==========================================
  Files         368      368              
  Lines       54083    54130      +47     
==========================================
+ Hits        38441    38624     +183     
+ Misses      13343    13210     -133     
+ Partials     2299     2296       -3     
Files Coverage Δ
regulation-worker/internal/delete/api/api.go 84.48% <100.00%> (+19.17%) ⬆️
services/oauth/oauth.go 85.25% <91.20%> (+28.47%) ⬆️
router/handle.go 70.75% <0.00%> (+0.25%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@fracasula fracasula left a comment

Choose a reason for hiding this comment

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

@sanpj2292 LGTM 👍 Has this been tested on dev?

@sanpj2292
Copy link
Contributor Author

@sanpj2292 LGTM 👍 Has this been tested on dev?

Yes. After completion of testing I have removed Dont Merge label

@fracasula
Copy link
Collaborator

@sanpj2292 LGTM 👍 Has this been tested on dev?

Yes. After completion of testing I have removed Dont Merge label

Great, thanks 👍

@fracasula fracasula merged commit a53a127 into master Oct 18, 2023
35 checks passed
@fracasula fracasula deleted the feat.oauth-access-denied-handling branch October 18, 2023 13:25
fracasula added a commit that referenced this pull request Oct 19, 2023
* fix: access_denied error handling for OAuth destinations

* fix: mock oauth service

Signed-off-by: Sai Sankeerth <sanpj2292@github.com>

* chore: rename ref_token_invalid_grant constant

Signed-off-by: Sai Sankeerth <sanpj2292@github.com>

* chore: update the method for authStatus toggle to PUT

* chore: include contract changes

* fix: add AUTH_STATUS_INACTIVE handling in regulation-worker

* chore: refactoring some changes, adding logic for handling invalid_grant error while refreshing the token

* fix: address comments
- include relevant status-codes
- remove unnecessary printf statements

* fix: send badrequest when required parameters are not sent in tests

* fix: change response error message for authStatusInactive req(both failure & success)
- refactor authStatus toggle handling in deleteUsers
- correction of test-cases in deleteUsers

* fix: add multiple go-routines tests for authStatus/toggle

* fix: formatting

* fix: rename variables, send right error message post inactivation of authStatus

* fix: comment correction

* fix: remove unused argument

* fix: updated wrong url status-code to 404

* fix: propagate error message rather than error type

* fix: unused return variable removal

* fix: remove secret info from cache after inactivating authStatus

* fix: rename oauth function & update related mock

---------

Signed-off-by: Sai Sankeerth <sanpj2292@github.com>
Co-authored-by: Sai Sankeerth <sanpj2292@github.com>
Co-authored-by: Francesco Casula <fracasula@users.noreply.github.com>
fracasula added a commit that referenced this pull request Oct 30, 2023
* fix: access_denied error handling for OAuth destinations

* fix: mock oauth service

Signed-off-by: Sai Sankeerth <sanpj2292@github.com>

* chore: rename ref_token_invalid_grant constant

Signed-off-by: Sai Sankeerth <sanpj2292@github.com>

* chore: update the method for authStatus toggle to PUT

* chore: include contract changes

* fix: add AUTH_STATUS_INACTIVE handling in regulation-worker

* chore: refactoring some changes, adding logic for handling invalid_grant error while refreshing the token

* fix: address comments
- include relevant status-codes
- remove unnecessary printf statements

* fix: send badrequest when required parameters are not sent in tests

* fix: change response error message for authStatusInactive req(both failure & success)
- refactor authStatus toggle handling in deleteUsers
- correction of test-cases in deleteUsers

* fix: add multiple go-routines tests for authStatus/toggle

* fix: formatting

* fix: rename variables, send right error message post inactivation of authStatus

* fix: comment correction

* fix: remove unused argument

* fix: updated wrong url status-code to 404

* fix: propagate error message rather than error type

* fix: unused return variable removal

* fix: remove secret info from cache after inactivating authStatus

* fix: rename oauth function & update related mock

---------

Signed-off-by: Sai Sankeerth <sanpj2292@github.com>
Co-authored-by: Sai Sankeerth <sanpj2292@github.com>
Co-authored-by: Francesco Casula <fracasula@users.noreply.github.com>
fracasula added a commit that referenced this pull request Oct 30, 2023
* fix: access_denied error handling for OAuth destinations

* fix: mock oauth service

Signed-off-by: Sai Sankeerth <sanpj2292@github.com>

* chore: rename ref_token_invalid_grant constant

Signed-off-by: Sai Sankeerth <sanpj2292@github.com>

* chore: update the method for authStatus toggle to PUT

* chore: include contract changes

* fix: add AUTH_STATUS_INACTIVE handling in regulation-worker

* chore: refactoring some changes, adding logic for handling invalid_grant error while refreshing the token

* fix: address comments
- include relevant status-codes
- remove unnecessary printf statements

* fix: send badrequest when required parameters are not sent in tests

* fix: change response error message for authStatusInactive req(both failure & success)
- refactor authStatus toggle handling in deleteUsers
- correction of test-cases in deleteUsers

* fix: add multiple go-routines tests for authStatus/toggle

* fix: formatting

* fix: rename variables, send right error message post inactivation of authStatus

* fix: comment correction

* fix: remove unused argument

* fix: updated wrong url status-code to 404

* fix: propagate error message rather than error type

* fix: unused return variable removal

* fix: remove secret info from cache after inactivating authStatus

* fix: rename oauth function & update related mock

---------

Signed-off-by: Sai Sankeerth <sanpj2292@github.com>
Co-authored-by: Sai Sankeerth <sanpj2292@github.com>
Co-authored-by: Francesco Casula <fracasula@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants