这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@wawhal
Copy link
Contributor

@wawhal wawhal commented Aug 7, 2019

Description

  • in CLI mode show error message on login page if no/invalid admin secret
  • allow not persisting admin secret in browser local storage on login page
  • admin secret value in graphiql headers is masked in local storage, its value is set to current admin secret when reading back from local storage
  • if admin secret is set and graphiql headers does not have the admin secret header set (typically happens when admin secret is just added) add a new admin secret header and set in local storage that this has been done. Now if user removes the admin secret header it will not be added back.
  • make graphiql introspection query only after headers have been set.
  • change routes from /metadata -> /settings
  • add separate logout page

Affected components

  • Console

Related Issues

#2660, #2663

Solution and Design

  • Rewrite login page and its associated actions. Add a checkbox to choose whether to persist the admin secret.
    • Persist: admin secret set to LS. admin secret is set in globals
    • Do not persist: admin secret is set in globals
  • implement getAdminSecret fn which fetches from localstorage or globals
  • On GraphiQL page load, dispatch header init:
    • read headers from LS. set actual admin secret value if admin secret header present
    • if admin secret is set and admin secret header is not present, add admin secret header and persist to LS that admin secret header wasAdded. Do not add admin secret header if admin secret header was already added once. ie. wasAdded value is set in LS.
    • persist headers to LS. mask admin secret value if header is present.
    • push headers to redux and mark headers are initialised as a flag
  • Introspection happens only after headers are initialised

Steps to test and verify

Cases:

  • With vs Without admin secret
  • Server vs CLI mode
  • Persisted vs Non-persisted admin secret
  • Prod vs Dev mode

Test:

  • Login behaviour
  • Logout behaviour
  • Graphiql headers
  • Data API headers

Limitations, known bugs & workarounds

@netlify
Copy link

netlify bot commented Aug 7, 2019

Deploy preview for hasura-docs ready!

Built with commit 05e16df

https://deploy-preview-2686--hasura-docs.netlify.com

@hasura-bot
Copy link
Contributor

Review app for commit 540c216 deployed to Heroku: https://hge-ci-pull-2686.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2686-540c2164

@wawhal wawhal requested review from rikinsk-zz August 8, 2019 10:25
@rikinsk-zz rikinsk-zz added the c/console Related to console label Aug 8, 2019
@rikinsk-zz rikinsk-zz marked this pull request as ready for review August 9, 2019 09:25
@tirumaraiselvan
Copy link
Contributor

Also, please make sure that when a role header is set then two queries are NOT made for introspection (first as admin, then as the role).

@hasura-bot
Copy link
Contributor

Review app for commit 4e416ce deployed to Heroku: https://hge-ci-pull-2686.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2686-4e416cec

Copy link

@rikinsk-zz rikinsk-zz left a comment

Choose a reason for hiding this comment

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

  • Admin secret is undefined. Something's wrong here
    Screenshot from 2019-08-21 18-55-33

@hasura-bot
Copy link
Contributor

Review app for commit d3a6869 deployed to Heroku: https://hge-ci-pull-2686.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2686-d3a68696

@hasura-bot
Copy link
Contributor

Review app for commit 506cc22 deployed to Heroku: https://hge-ci-pull-2686.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2686-506cc229

@wawhal wawhal added the s/do-not-merge Do not merge this pull request to master label Aug 22, 2019
@hasura-bot
Copy link
Contributor

Review app for commit 7312c33 deployed to Heroku: https://hge-ci-pull-2686.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2686-7312c335

@rikinsk-zz rikinsk-zz self-requested a review August 26, 2019 10:56
@hasura-bot
Copy link
Contributor

Review app for commit cc22c8c deployed to Heroku: https://hge-ci-pull-2686.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2686-cc22c8cb

@wawhal wawhal requested a review from rikinsk as a code owner August 29, 2019 07:42
@hasura-bot
Copy link
Contributor

Review app for commit 5a01ac5 deployed to Heroku: https://hge-ci-pull-2686.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2686-5a01ac5d

@hasura-bot
Copy link
Contributor

Review app for commit b3b6b06 deployed to Heroku: https://hge-ci-pull-2686.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2686-b3b6b06c

@rikinsk rikinsk removed the s/do-not-merge Do not merge this pull request to master label Sep 24, 2019
@rikinsk
Copy link
Member

rikinsk commented Sep 25, 2019

/heroku deploy

@hasura-bot
Copy link
Contributor

Review app for commit d0b3fd9 deployed to Heroku: https://hge-ci-pull-2686.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2686-d0b3fd9a

@hasura-bot
Copy link
Contributor

Review app for commit 05e16df deployed to Heroku: https://hge-ci-pull-2686.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2686-05e16df9

@rikinsk rikinsk merged commit 1d1de94 into hasura:master Sep 25, 2019
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-2686.herokuapp.com is deleted

@rikinsk
Copy link
Member

rikinsk commented Sep 27, 2019

/heroku deploy

@hasura-bot
Copy link
Contributor

Review app for commit 05e16df deployed to Heroku: https://hge-ci-pull-2686.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2686-05e16df9

@rikinsk
Copy link
Member

rikinsk commented Sep 27, 2019

/heroku destroy

@hasura-bot
Copy link
Contributor

@rikinsk not a valid heroku command (deploy, delete)

@rikinsk
Copy link
Member

rikinsk commented Sep 27, 2019

/heroku delete

@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-2686.herokuapp.com is deleted

polRk pushed a commit to polRk/graphql-engine that referenced this pull request Feb 12, 2020
* change login flow to handle admin secret persistence

* handle headers init state

* add tooltip for remember-me

* remove log, make label clickable

* fix a closure scope bug

* handle login verification at route level

* update Login.js

* refactor

* remove extra file

* refactor

* add id to tooltips

* remove adminsecretlabel + update admin secret storage flow

* fix heartIcon close handling

* .

* fix admin secret setting

* fix urlPrefix

* add admin secret header if not present

* update jwt analyzer icon

* persist if admin secret header has already been added

* set cli console mode as constant

* handle CLI admin secret errors

* make separate logout page

* fix typos

* fix typos

* fix typos

* fix typos

* fix cli error

* fix login page path
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c/console Related to console

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants