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

improper error messaging when setting API key expiry to 1 day #895

Open
5 tasks done
josefaidt opened this issue Oct 18, 2022 · 3 comments
Open
5 tasks done

improper error messaging when setting API key expiry to 1 day #895

josefaidt opened this issue Oct 18, 2022 · 3 comments
Assignees
Labels
api-graphql bug Something isn't working good first issue Good for newcomers p2

Comments

@josefaidt
Copy link
Contributor

Before opening, please confirm:

  • I have installed the latest version of the Amplify CLI (see above), and confirmed that the issue still persists.
  • I have searched for duplicate or closed issues.
  • I have read the guide for submitting bug reports.
  • I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.
  • I have removed any sensitive information from my code snippets and submission.

How did you install the Amplify CLI?

pnpm

If applicable, what version of Node.js are you using?

18

Amplify CLI Version

10.2.3

What operating system are you using?

mac

Did you make any manual changes to the cloud resources managed by Amplify? Please describe the changes made.

n/a

Amplify Categories

Not applicable

Amplify Commands

Not applicable

Describe the bug

When setting the API key expiry to 1 day, we are prompted with an error at the very end of the update api flow. The prompt allows input from 1-365, which should allow 1

aws-amplify/reproductions/809 
➜  amplify update api
? Select from one of the below mentioned services: GraphQL

General information
- Name: 809

Authorization modes
- Default: API key

Conflict detection (required for DataStore)
- Disabled

? Select a setting to edit Authorization modes
? Choose the default authorization type for the API API key
✔ Enter a description for the API key: · 
✔ After how many days from now the API key should expire (1-365): · 1
? Configure additional auth types? Yes
? Choose the additional authorization types you want to configure for the API Amazon Cognito User Pool, IAM
Cognito UserPool configuration
Using service: Cognito, provided by: awscloudformation
 
 The current configured provider is Amazon Cognito. 
 
 Do you want to use the default authentication and security configuration? Default configuration
 Warning: you will not be able to edit these selections. 
 How do you want users to be able to sign in? Username
 Do you want to configure advanced settings? No, I am done.
✅ Successfully added auth resource 809bca5b0dc locally

✅ Some next steps:
"amplify push" will build all your local backend resources and provision it in the cloud
"amplify publish" will build all your local backend and frontend resources (if you have hosting category added) and provision it in the cloud


⚠️  WARNING: your GraphQL API currently allows public create, read, update, and delete access to all models via an API Key. To configure PRODUCTION-READY authorization rules, review: https://docs.amplify.aws/cli/graphql/authorization-rules

🛑 API key expiration must be between 1 and 365 days.

Expected behavior

I can set the API key expiry to 1 day

Reproduction steps

  1. amplify init -y
  2. amplify add api
  3. choose GraphQL
  4. select authorization modes to change API key
  5. input 1 day for expiry
  6. observe error after completing prompts

GraphQL schema(s)

# Put schemas below this line

Log output

# Put your logs below this line


Additional information

No response

@ykethan
Copy link

ykethan commented Oct 18, 2022

Was able to reproduce the issue by setting API key expiry to 1 day. marking this as bug.

@ykethan ykethan added bug Something isn't working and removed pending-triage labels Oct 18, 2022
@josefaidt josefaidt added p2 good first issue Good for newcomers labels Oct 18, 2022
@basilzuberi
Copy link

basilzuberi commented Oct 19, 2022

Hi there!
This would be my first open source contribution so I was looking into this. Changing Duration.days(1) to Duration.days(0) within graphql-api.ts makes this specific scenario work but then inputting 0 also "passes" through successfully which is undesired behaviour. I'm looking into this further, if anyone else has stuff they can add let me know :)

General information
- Name: test

Authorization modes
- Default: API key

Conflict detection (required for DataStore)
- Disabled

? Select a setting to edit Authorization modes
? Choose the default authorization type for the API API key
✔ Enter a description for the API key: · test
✔ After how many days from now the API key should expire (1-365): · 0
? Configure additional auth types? No

⚠️  WARNING: your GraphQL API currently allows public create, read, update, and delete access to all models via an API Key. To configure PRODUCTION-READY authorization rules, review: https://docs.amplify.aws/cli/graphql/authorization-rules

✅ GraphQL schema compiled successfully.
✅ Successfully updated resource

@basilzuberi
Copy link

Update: The issue seems to stem from the fact that one full day (24 hours) makes the condition fail. In the if statement I tried the following and these work:

config?.expires?.isBefore(Duration.hours(23))
config?.expires?.isBefore(Duration.days(1).minus(Duration.seconds(1))) // basically taking one day and subtracting a second

this does not work:
config?.expires?.isBefore(Duration.hours(24))
config?.expires?.isBefore(Duration.days(1))

Also.. passing in 0 regardless makes the tests pass, this is due to the nature of the Duration class... doing a console log when passing in 0 gives an "Unknown" and I guess this would be another check to do.

If there's any suggestions about how to go through with it let me know, I can start a commit if you assign this to me :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-graphql bug Something isn't working good first issue Good for newcomers p2
Projects
None yet
Development

No branches or pull requests

4 participants