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

renders local login form if and only if you're using it #34

Merged
merged 1 commit into from
Jan 5, 2021

Conversation

felkerch
Copy link
Contributor

@felkerch felkerch commented Jan 4, 2021

Description of Changes

Updates the login view to render the local login form only if it's one of the auth providers.

Steps to Test

  1. Local setup:
  • In amphora-auth: npm i && npm link
  • In amphora: npm i && npm link && npm link amphora-auth
  • In your app: npm link amphora
  1. To test, modify your amphora instantation:
const providers = ['google', 'local'];
amphora({
      app,
      renderers,
      providers,
      sessionStore,
      storage,
      eventBus,
      plugins: [amphoraSearchPlugin, amphoraSchedule],
      bootstrap: bootstrapOnStart && isBootstrapProcess
    });
  1. Add 'local' to this list and verify that you see the login form. Remove it and verify that you don't.

Copy link
Contributor

@mattoberle mattoberle left a comment

Choose a reason for hiding this comment

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

This looks good to me!

I added a test locally to index.test.js that looked like this and removed the .not

    it('returns useLocalAuth=true when the local provider is enabled', function () {
      fn({ path: '' }, ['local'])({ flash: _noop }, mockRes);                    
      fn({ path: '' }, [])({ flash: _noop }, mockRes);                           
      expect(mockRes.send.mock.calls[0]).not.toEqual(mockRes.send.mock.calls[1]);
    });

The diff in the rendered template is what I'd expect.

2021-01-05_102500

@mattoberle mattoberle merged commit 85319ec into clay:master Jan 5, 2021
@mattoberle
Copy link
Contributor

@felkerch your PR is merged and I'm going to release a couple of versions.

The v1.3.x versions of amphora-auth implement the "secure" flag on the session cookie.
When running Clay behind a proxy this may require some additional work.

From what I understand, not everyone is ready to adopt that flag yet.

As a result, I'm going to release v1.2.1 which backports this PR but excludes the secure cookie change.
That release will be cut from the v1.2.x branch.

I'm also going to release v1.3.2 which will be cut from the master branch.

@felkerch
Copy link
Contributor Author

felkerch commented Jan 6, 2021

Thanks @mattoberle!

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.

2 participants