Skip to content
This repository has been archived by the owner on May 31, 2019. It is now read-only.

Top level auth config for server #54

Merged
merged 6 commits into from
Aug 21, 2018
Merged

Top level auth config for server #54

merged 6 commits into from
Aug 21, 2018

Conversation

wtrocki
Copy link
Contributor

@wtrocki wtrocki commented Aug 15, 2018

Motivation

Branch that is going to be used for collaboration for

NOTES: Pushed helper files that should not land into master only to help and point out preconfigured application cluster. They going to be removed after

Verification steps

Verification will focus on memeolist example.
We will need to run memeolist without keycloak

  1. npm run dev:memeo >
  2. Log should appear: Keycloak authentication is not configured
  3. UI queries should work

Memeolist with keycloak.

  1. npm run dev:keymemeo > Initializing Keycloak authentication
  2. UI queries should not work - keycloak login page should appear (if you provide redirect url :)

@aliok
Copy link
Contributor

aliok commented Aug 15, 2018

The direction of this PR looks good.

@wtrocki
Copy link
Contributor Author

wtrocki commented Aug 15, 2018

@darahayes Actually you may be right. We may not need session for our needs. It's up to investigation.
Going to use this with simple end to end server to give us prototype that we can extend over the time.
This code is nowhere close to be merged, but it is going to be used to facilitate work on clients and UI

@wtrocki wtrocki requested a review from ziccardi August 16, 2018 10:09
@wtrocki
Copy link
Contributor Author

wtrocki commented Aug 17, 2018

Verified that this is working and waiting for @StephenCoady merge to integrate this into Apollo 2.0

@wtrocki wtrocki changed the title WIP: Basic config for keycloak server Top level auth config for server Aug 17, 2018
@coveralls
Copy link

coveralls commented Aug 21, 2018

Pull Request Test Coverage Report for Build 522

  • 16 of 27 (59.26%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+36.2%) to 82.662%

Changes Missing Coverage Covered Lines Changed/Added Lines %
server/config/index.js 2 7 28.57%
server/security/index.js 6 12 50.0%
Totals Coverage Status
Change from base Build #446: 36.2%
Covered Lines: 830
Relevant Lines: 970

💛 - Coveralls

const port = process.env.HTTP_PORT || '8000'

const config = {
server: {
apiPath: topLevelGraphqlPath,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you rename this to graphqlEndpoint and move it under the graphqlConfig object?

@@ -28,17 +28,19 @@ if (process.env.PLAYGROUND_VARIABLES_FILE) {
}
}

const topLevelGraphqlPath = '/graphql'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you rename this to graphqlEndpoint?

@darahayes darahayes merged commit 1c4190e into master Aug 21, 2018
@darahayes darahayes deleted the keycloak-auth branch August 21, 2018 15:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants