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

(aws-cdk/aws-amplify): basicAuth cannot be removed #15028

Closed
mwiarda-fette opened this issue Jun 8, 2021 · 4 comments · Fixed by #15243
Closed

(aws-cdk/aws-amplify): basicAuth cannot be removed #15028

mwiarda-fette opened this issue Jun 8, 2021 · 4 comments · Fixed by #15243
Assignees
Labels
@aws-cdk/aws-amplify Related to AWS Amplify bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@mwiarda-fette
Copy link

mwiarda-fette commented Jun 8, 2021

Hi,

basicAuth can be enabled on an amplify application using the cdk.
Unfortunately, that setting can not be reversed. The cloudformation template changes but basic auth is still active in amplify.

Reproduction Steps

  1. Create new amplify app with basic auth:
import * as codebuild from '@aws-cdk/aws-codebuild';
import * as amplify from '@aws-cdk/aws-amplify';
import * as cdk from '@aws-cdk/core';

const amplifyApp = new amplify.App(this, 'MyApp', {
  basicAuth: amplify.BasicAuth.fromCredentials("admin", this.secret.secretValueFromJson("password")),
  sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
    owner: '<user>',
    repository: '<repo>',
    oauthToken: cdk.SecretValue.secretsManager('my-github-token')
  }),
  buildSpec: codebuild.BuildSpec.fromObjectToYaml({ // Alternatively add a `amplify.yml` to the repo
    version: '1.0',
    frontend: {
      phases: {
        preBuild: {
          commands: [
            'yarn'
          ]
        },
        build: {
          commands: [
            'yarn build'
          ]
        }
      },
      artifacts: {
        baseDirectory: 'public',
        files: '**/*'
      }
    }
  })
});
  1. Website should now be password protected
  2. Remove line basicAuth: amplify.BasicAuth.fromCredentials("admin", this.secret.secretValueFromJson("password"))
  3. App is still password protected

What did you expect to happen?

Password protection should be removed

What actually happened?

Template changes (shows in cdk diff) but protection not removed in amplify


This is 🐛 Bug Report

@mwiarda-fette mwiarda-fette added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 8, 2021
@github-actions github-actions bot added the @aws-cdk/aws-amplify Related to AWS Amplify label Jun 8, 2021
@skinny85
Copy link
Contributor

Hi @mwiarda-fette,

thanks for opening the issue. Do you perhaps have Premium Support? I think this might be worth it to open to the Amplify team directly - looks like their CloudFormation support does not handle edits correctly. Unfortunately, there's not much we can do here from the CDK side.

Thanks,
Adam

@skinny85 skinny85 added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jun 17, 2021
@jogold
Copy link
Contributor

jogold commented Jun 18, 2021

There's maybe something to do with https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-amplify-app-basicauthconfig.html#cfn-amplify-app-basicauthconfig-enablebasicauth?

@mwiarda-fette
Copy link
Author

mwiarda-fette commented Jun 18, 2021

Hi Adam, thanks for the feedback.
Unfortunately, we only have basic support.
I will contact our account manager. Maybe he would like to open a case.

Best,
Michael

jogold added a commit to jogold/aws-cdk that referenced this issue Jun 22, 2021
Removing the `BasicAuthConfig` property from the template doesn't remove
the basic auth. Explicitely set `EnableBasicAuth` to `false` instead.

Closes aws#15028
@mergify mergify bot closed this as completed in #15243 Jun 22, 2021
mergify bot pushed a commit that referenced this issue Jun 22, 2021
Removing the `BasicAuthConfig` property from the template doesn't remove
the basic auth. Explicitely set `EnableBasicAuth` to `false` instead.

Closes #15028


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Seiya6329 pushed a commit to Seiya6329/aws-cdk that referenced this issue Jun 22, 2021
Removing the `BasicAuthConfig` property from the template doesn't remove
the basic auth. Explicitely set `EnableBasicAuth` to `false` instead.

Closes aws#15028


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
matthewsvu pushed a commit to matthewsvu/aws-cdk that referenced this issue Jun 22, 2021
Removing the `BasicAuthConfig` property from the template doesn't remove
the basic auth. Explicitely set `EnableBasicAuth` to `false` instead.

Closes aws#15028


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
Removing the `BasicAuthConfig` property from the template doesn't remove
the basic auth. Explicitely set `EnableBasicAuth` to `false` instead.

Closes aws#15028


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-amplify Related to AWS Amplify bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants