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

spar update issuer #1065

Merged
merged 52 commits into from
Apr 21, 2020
Merged

spar update issuer #1065

merged 52 commits into from
Apr 21, 2020

Conversation

fisx
Copy link
Contributor

@fisx fisx commented Apr 17, 2020

https://github.com/zinfra/backend-issues/issues/929

this took about an hour to hack together, and the only thing that's missing is two functions for lookup users on old issuers and for moving them to the new one. and tests. and i did some cleanup on the way. to be specific:

  • changed IdP type alias to carry TeamId and [Isuser].
  • change db schema to match new type.
  • type checker now found some IdPConfig Teamid where the type alias IdP should be used.
  • change errors: when trying to update issuer to one that already exists, that's always bad, not only if it's in another team!

now i should probably spend another hour implementing the competing idea of keeping two idps, adding cassandra columns to make the old and the new idp reference each other, and allow for concurrent use of both. right now my gut feeling is that it'll be a lot more work to get right.

UPDATE: I just did them both in one PR, seemed easiest. :)

@fisx fisx force-pushed the fisx/spar-update-issuer-v1 branch from a17a586 to dbaa123 Compare April 20, 2020 07:37
@fisx fisx force-pushed the fisx/spar-update-issuer-v1 branch from 728a8a9 to 951b251 Compare April 20, 2020 10:46
@fisx
Copy link
Contributor Author

fisx commented Apr 20, 2020

things to think about more:

  • two devices, one using the old login code, one using the new one?
  • are login codes cached?
  • when do users get logged out and forced to login again?

@fisx fisx force-pushed the fisx/spar-update-issuer-v1 branch from c5cb31e to 60b8b3d Compare April 20, 2020 12:32
@fisx fisx force-pushed the fisx/spar-update-issuer-v1 branch from a75c138 to aa58abc Compare April 20, 2020 14:23
@fisx fisx changed the title spar update issuer [WIP] spar update issuer Apr 20, 2020
@fisx
Copy link
Contributor Author

fisx commented Apr 20, 2020

Any C* experts who would like to comment on e2dddeb? @jschaul?

@fisx
Copy link
Contributor Author

fisx commented Apr 20, 2020

Any C* experts who would like to comment on e2dddeb? @jschaul?

(I'm happy to give context in a quick phone call tomorrow.)

@fisx fisx requested a review from arianvp April 20, 2020 20:07
@fisx fisx changed the title [WIP] spar update issuer spar update issuer Apr 20, 2020
@fisx fisx mentioned this pull request Apr 21, 2020
services/spar/schema/src/V9.hs Outdated Show resolved Hide resolved
services/spar/src/Spar/API.hs Show resolved Hide resolved
services/spar/src/Spar/API.hs Show resolved Hide resolved
services/spar/src/Spar/API.hs Show resolved Hide resolved
services/spar/src/Spar/API.hs Show resolved Hide resolved
@fisx fisx changed the title spar update issuer [WIP] spar update issuer Apr 21, 2020
@fisx fisx changed the title [WIP] spar update issuer spar update issuer Apr 21, 2020
@fisx fisx merged commit d387164 into develop Apr 21, 2020
@fisx fisx mentioned this pull request Apr 21, 2020
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.

None yet

3 participants