-
Notifications
You must be signed in to change notification settings - Fork 52
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
Federated Auth #151
base: master
Are you sure you want to change the base?
Federated Auth #151
Conversation
The login packet is where I'm stuck. seems to be quite complicated to get the offsets and so on right |
Thank you for submitting this pull request. I need following in order to merge it:
|
@@ -220,6 +221,17 @@ class PacketType: | |||
TDS_FOLEDB = 0x10 | |||
TDS_FREADONLY_INTENT = 0x20 | |||
|
|||
# TDS Login Features |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Feature Extensions
As part of the Feature extension negotiations, SQL server also provides the Audience for which the Access token should be provided, along with bunch of other information. Is it possible to bake that into the APIs, so that the users can use the audience of the token, negotiated with the SQL server? Or do you think that's orthogonal to this PR and I should open a different issue? |
@@ -1323,6 +1341,9 @@ def parse_prelogin(self, octets, login): | |||
elif type_id == PreLoginToken.INSTOPT: | |||
# ignore instance name mismatch | |||
pass | |||
elif type_id == PreLoginToken.FEDAUTHREQUIRED: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect that with this change, even Sql User/Password auth will start erroring out, if the access_token is not set.
You need to check if the payload has a 0 or a 1 value, which are considered valid.
If the server sends a 0x01 AND if the Access_token is set, then only can we send the access token during login negotiation.
@@ -220,6 +221,17 @@ class PacketType: | |||
TDS_FOLEDB = 0x10 | |||
TDS_FREADONLY_INTENT = 0x20 | |||
|
|||
# TDS Login Features |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit:
# TDS Login Features | |
# TDS Login Feature Extensions |
I have implemented and created #166 |
Would fix #135