Comments (9)
Is step 3 about changing the username in Keycloak itself?
Am I correct in understanding that the DELETE method would need to be implemented legacy-side to do something so that the GET methods no longer return that user?
If so, the solution makes sense to me. I'm a little wary of using DELETE for this, but I suppose it is the easiest way to do this.
An alternative would probably be treating a migration as a REST "resource" and using POST on some "user migration" endpoint to mark it but that might be overkill. I'm open to discussion if you want to ponder this further. Otherwise, DELETE isn't too bad and I will definitely appreciate the bug fix :)
from keycloak-user-migration.
Step 3: Yes, changing the username in Keycloak.
Yes, the intention is to prevent GET/POST from returning 200 status codes for the username in future requests.
In my team we discussed whether DELETE vs POST vs PUT for quite a while and finally settled on DELETE.
Since the logic for handling the delete resides in the custom legacy API it would be up to the implementer's discretion whether they want to actually delete the record(s) for the username or update their records to respond with non-200 responses to future GET/POST requests for the username.
Essentially we argued ourselves into DELETE, since the behavior from keycloak-user-migration's perspective is that GET/POST requests in the future for the username will never respond with a status code of 200 again (ie: deleted).
Once #9 is merged, I'll open a PR for this feature. I'm definitely open to use something other than DELETE if you prefer, I'd just need to know the spec (ie: POST with { markMigrated: true }
or something).
from keycloak-user-migration.
Since you've discussed this with your team, DELETE is fine and will probably be easier to explain in the docs.
One thing I ask for the PR is that you document not only the "how", but also the "why" in the readme, so that everyone using the plugin is fully aware of the fact that they must handle a DELETE and the reasons behind it (maybe even a nice RuntimeException could remind them if DELETE doesn't return 200...?).
If I'm correct, the DELETE is not needed if the username can never change, so maybe we could make it optional? I'm thinking of a toggle in the settings for whether the username can change (or could we check that from realm properties...?) and if it's on then the client has to implement DELETE, otherwise you can just go about your day with the standard set of endpoints. Again, just thinking out loud and not something I'm adamant about - what's your opinion on this?
Again, though, big thanks for the contribution! This is going to save people some headaches (me included).
from keycloak-user-migration.
One of the reasons we argued ourselves to using DELETE was that it would allow most legacy APIs to operate unchanged -- if well designed to serve a 400 (bad request) on an unexpected method.
However, since this tool is in production use and legacy APIs may not be well designed (I've seen servers fail to close connections on unexpected requests), I think adding a toggle is a fantastic idea. I'd lean towards defaulting the toggle to off and documenting in the DELETE README that the toggle needs to be enabled and the REST API needs to handle the request if usernames may be changed within Keycloak.
With this, I think we can avoid needing to throw a RuntimeException since the feature would be opt in.
from keycloak-user-migration.
Great! It seems we're on the same page, then. I think the DELETE documentation would fit neatly in the Optional - additional configuration
section of README.md.
The toggle being off by default is definitely a good idea, IMO.
I see the RuntimeException as a last resort kind of thing - I imagine a user of the plugin skipping the README.md (admittedly a bad idea), noticing the toggle and thinking that's all he needs to support username changes. Then he gets an error message like A DELETE request didn't return HTTP 200. Make sure to implement the endpoint to enable support for changing usernames
(in the logs). I'll leave that one up to you, though.
from keycloak-user-migration.
Did this stall out? What is remaining for this to close? I see #9 got merged. But a follow-up PR does not seem to be linked here.
from keycloak-user-migration.
Sorry @heddn, my team was under a time crunch, so we just slammed DELETE in without the toggle. I've since not been the one maintaining the Keycloak install so I lost track of it.
from keycloak-user-migration.
@codycraven I'm happy without the toggle as a PR. We don't need it disabled. If that code is still sitting around, maybe someone could post it up? Adding a small tweak to toggle off/on might be easier then building the whole thing.
from keycloak-user-migration.
@heddn I've created and closed a PR (#48) for you to reference the code.
I closed the PR since I know it's not the desired solution and I don't have test coverage for it. It would be great if you'd work off this and add documentation for DELETE, the toggle, and a test or two so that future users will have the solution available since you have a project at hand to verify it with.
from keycloak-user-migration.
Related Issues (20)
- Compatible with Keycloak 22 (JAX-RS)? HOT 10
- Not able to recreate migrated users HOT 1
- Swagger is not working HOT 1
- Rest Client URI is not invoked. Nothing is logs related to this plugin HOT 6
- Federation Cache Expiration Issues and Docs Clarification HOT 4
- Username with uppercase letters causing errors HOT 8
- Support fort 23.X HOT 3
- Fallback to Provider Password if Keycloak internal password is wrong HOT 4
- Mobile Number support? HOT 2
- Docker is required?? HOT 1
- migration via rest doesnt show up in user federation HOT 2
- rest api doesnt import wp users HOT 1
- [BUG] Resetting password randomly locks user out of Keycloak, returning 403 for "GET /admin/serverinfo" request HOT 1
- No defense against brute force attacks [enhancement idea] HOT 3
- Bypassing password complexity requirements on import HOT 1
- End-to-end tests are broken
- Half migrated users can not be deleted in Keycloak HOT 1
- FR: Credentials import
- Issue .. Cannot create user/delete user then recreate user (email in use ) unless migrator is disabled!
- Design Question HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from keycloak-user-migration.