Feat add possibility to use Bearer token #1023
Conversation
|
So, this If this is something specific to your installation of netbox, and a proxy that you have set up to access your specific netbox that requires a bearer token, I don't know that we will want to accept this patch since it is specific to your environment |
Hello @sc68cal , |
|
I think this is a valid PR. @F-PMOR Could you just run black to make the formatting ok? Just run |
Ok, Thanks @rodvand , I will do it today, I'll send you the result or my miss knowledge of the process... Thank you @sc68cal, for the exchange that permit me to update the comments to argue the RF, Regards |
|
Hello @rodvand, check pass successfully, does I have something else do to ? |
|
This needs documentation added so that people understand how to utilize |
Thanks @sc68cal , where can I put documentation ? It's my first PR and I'm a little bit lost. I put some information to add to the documentation in my PR. Is it enought ? |
|
No, you need to at the very least add it to the documentation for the |
|
Honestly I think this needs some work. I think I would rather see Basically I don't like the string manipulation that you are doing, and would prefer it to be something like: self.headers.update({"Authorization": f"{{token.type}} {{token.value}}"})So that we could support the following in configuration: and |
Yes, I agree, it's better, but I'm not able to do that ! I ever don't know what's happen and how works the CI linter for Documentation that doesn't pass. |
|
Yep that's fair, I will see if I can add commits to your PR, and then do some research to try and accomplish what I was thinking |
This is a bit of a hack, where we set they type to "Token" and provide a token, so we exercise the new codepath of having token be a dictionary, but without attempting to test a new authentication type like Bearer, which we do not have in CI
This is to try and loosen the constraint, and see if we can accept both a dictionary and a string.
Add support for a new token type for nb_inventory
sc68cal
dismissed
their stale review
Made the changes I was looking for, but need approval from another maintainer now that I've made changes
|
@rovand This can probably be squashed into a single commit and just add me as a co-author via a message at the bottom of the commit message:
|
Important information
this is my first contribution for an open source project. I'm not a full dev and have many withdraw with testing process, so I didn't do it with pytest, but I do manual test on my project. So I think some change will have to be done by expert, sorry and thank's for your understanding.
Related Issue
id : 1014
New Behavior
This feature add the possibility to change the type of token that has to be send to netbox or to a proxy behind netbox.
If the type of token is specified as Bearer (or bearer), it's send as a Bearer token
...
Contrast to Current Behavior
A check is done to verify the type of token configured in the ansible inventory file. If Bearer or bearer key is found, then Bearer is added before the key to send in Authorization headers. However, Token is the keyword send as usual.
...
Discussion: Benefits and Drawbacks
Benefit :
drawbacks
Changed has been done cause I use a security proxy behind Netbox that accept only Bearer Token.
For security reason, Token as to be renewed often, this is not managed by this feature change.
It can be done by updating the inventory file.
...
Changes to the Documentation
Some information like below should be added to documentation, but I don't know where ?
In the inventory file :
Token : (string) should contain the token used to connect to netbox. If keyword Bearer is added at the beginning of the token, the plugins will use Bearer token type instead of standard Token type.
exemple :
token: thisismytoken # Token of type : Token
token: Bearer thisismytoken # token of type : Bearer
token: bearer thisismytoken # token of type : Bearer
...
Proposed Release Note Entry
Add Bearer token type.
...
Double Check
develbranch.