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

[#ENTE-82] Limit API access for incomplete services #116

Merged
merged 9 commits into from
May 14, 2021

Conversation

BurnedMarshal
Copy link
Contributor

@BurnedMarshal BurnedMarshal commented Apr 28, 2021

List of Changes

If the feature flag FF_DISABLE_INCOMPLETE_SERVICES is true only services matching that ValidService type can use:

  • CreateMessage
  • GetLimitedProfile
  • GetLimitedProfileByPOST
  • GetSubscriptionFeed
    Some Services added into FF_INCOMPLETE_SERVICE_WHITELIST can bypass ValidService check.

Motivation and Context

A Services that haven't the required metadata information can't be enabled to use some API. We need to specify through the feature flag when this filtering is enabled and a whitelist of services that don't need this check.

How Has This Been Tested?

unit test
test with io-mock

Screenshots (if appropriate):

Types of changes

  • Chore (nothing changes by a user perspective)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@pagopa-github-bot
Copy link
Contributor

pagopa-github-bot commented Apr 30, 2021

Warnings
⚠️

Please include a Pivotal story at the beginning of the PR title (see below).

Example of PR titles that include pivotal stories:

  • single story: [#123456] my PR title
  • multiple stories: [#123456,#123457,#123458] my PR title

Generated by 🚫 dangerJS

@codecov
Copy link

codecov bot commented Apr 30, 2021

Codecov Report

Merging #116 (0558b74) into master (cad0523) will increase coverage by 1.59%.
The diff coverage is 90.16%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #116      +/-   ##
==========================================
+ Coverage   81.82%   83.42%   +1.59%     
==========================================
  Files          27       27              
  Lines         886      899      +13     
  Branches       89       92       +3     
==========================================
+ Hits          725      750      +25     
+ Misses        160      148      -12     
  Partials        1        1              
Impacted Files Coverage Δ
GetSubscriptionsFeed/utils.ts 94.59% <ø> (ø)
WebhookNotificationActivity/client.ts 100.00% <ø> (ø)
utils/healthcheck.ts 41.02% <ø> (ø)
utils/responses.ts 90.90% <ø> (ø)
utils/subscription.ts 100.00% <ø> (ø)
GetLimitedProfileByPOST/handler.ts 68.75% <60.00%> (-14.59%) ⬇️
CreateMessage/handler.ts 78.76% <66.66%> (+15.29%) ⬆️
GetLimitedProfile/handler.ts 70.58% <66.66%> (-13.29%) ⬇️
utils/config.ts 73.33% <80.00%> (+0.60%) ⬆️
GetSubscriptionsFeed/handler.ts 85.71% <83.33%> (+0.80%) ⬆️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 039f75c...0558b74. Read the comment docs.

@BurnedMarshal BurnedMarshal marked this pull request as ready for review May 6, 2021 13:47
@BurnedMarshal BurnedMarshal force-pushed the ENTE-82-disable-api-incomplete-services branch from 4708aa6 to b687701 Compare May 6, 2021 13:53
Comment on lines 434 to 437
.map(_1 => true)
.mapLeft(
_1 => ResponseErrorForbiddenNotAuthorizedForRecipient
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.map(_1 => true)
.mapLeft(
_1 => ResponseErrorForbiddenNotAuthorizedForRecipient
)
.bimap(
ResponseErrorForbiddenNotAuthorizedForRecipient,
_1 => true
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in af84ae2

userAttributes.service.authorizedRecipients,
fiscalCode
)
).mapLeft(_ => ResponseErrorForbiddenNotAuthorizedForRecipient)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
).mapLeft(_ => ResponseErrorForbiddenNotAuthorizedForRecipient)
).mapLeft(ResponseErrorForbiddenNotAuthorizedForRecipient)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mapLeft need a function and ResponseErrorForbiddenNotAuthorizedForRecipient is not

utils/profile.ts Outdated
Comment on lines 126 to 127
.map(_1 => true)
.mapLeft(_1 => ResponseErrorForbiddenNotAuthorizedForRecipient)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.map(_1 => true)
.mapLeft(_1 => ResponseErrorForbiddenNotAuthorizedForRecipient)
.bimap(ResponseErrorForbiddenNotAuthorizedForRecipient,
_1 => true
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in af84ae2

@BurnedMarshal BurnedMarshal requested a review from AleDore May 10, 2021 08:02
@BurnedMarshal BurnedMarshal force-pushed the ENTE-82-disable-api-incomplete-services branch from af84ae2 to 516710d Compare May 14, 2021 08:08
@BurnedMarshal BurnedMarshal merged commit 7c1e65a into master May 14, 2021
@BurnedMarshal BurnedMarshal deleted the ENTE-82-disable-api-incomplete-services branch May 14, 2021 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants