[Revival] Support native authentication #2077
Conversation
Fixes jdalrymple#1098 These are the issues I encountered - please help: - [ ] fix TODOs - [ ] lint & fix (for me, there were a lot of unrelated changes after running lint:fix, so I didn't.) - [ ] verify this works because I couldn't test it because of jdalrymple#1105 Signed-off-by: Kipras Melnikovas <[email protected]>
Signed-off-by: Kipras Melnikovas <[email protected]>
it's good that I'm able to test on other environments too (this was needed on self-hosted GitLab Community edition) Signed-off-by: Kipras Melnikovas <[email protected]>
Signed-off-by: Kipras Melnikovas <[email protected]>
This reverts commit 3a8c02b. Signed-off-by: Kipras Melnikovas <[email protected]>
Signed-off-by: Kipras Melnikovas <[email protected]>
Signed-off-by: Kipras Melnikovas <[email protected]>
Signed-off-by: Kipras Melnikovas <[email protected]>
|
Hi @jdalrymple, I've recreated a PR to have the native-auth landed. |
|
@toniopelo Ill give it a look! This weeks just been madness. Ill follow up ASAP |
| /** | ||
| * TODO - what do I do here with the additionalBody? | ||
| * | ||
| * Update: attempting to parse `body` like so: |
There was a problem hiding this comment.
How is the additional information sent when there is a form data request? Should they be added to the formdata? If so this modification will probably have to happen in the specific requesters (Ky and Got) because they handle formdata differently
There was a problem hiding this comment.
hmm i probably never encountered this case or something because it was never an issue, but how to solve it - no clue..
edit: nvm, i remember that when i tried changing it (like u can see in the remaining comment) it broke things, and the current solution works. dunno why/how tho
There was a problem hiding this comment.
Did you ever try to import or export projects using the api? Thats really the only time the formdata is called on.
There was a problem hiding this comment.
nope, only used gitbeaker inside my own extension [1] but nowhere else..
|
Unit tests are good, everything seems to work BUT, i want to make sure the integration tests are good to go. Give me a little more time to fix up the pipeline 🙏 |
Glad i waited. When performing get requests, are these new properties still included in the body OR are they sent via query parameters? Is the header argument not sufficient enough? Why pass it through the body at all? |
|
Following up on my last question, anyone? |
|
Sorry @jdalrymple I lost momentum on this one. I'm unfortunately overbooked lastly but I'll try my best to give it some times in the following weeks. If @kiprasmel feels like doing it, you can obviously close my PR and reopen the old one (or continue on this one but not sure of the push rights on my branch). Really sorry guys 🙏 |
|
No worries! Take your time. |
Fixes #1098
This PR is a revival of this one from @kiprasmel, big thanks to him!