Skip to content
This repository has been archived by the owner on Mar 13, 2022. It is now read-only.

Correctly pad oidc tokens #70

Closed
wants to merge 2 commits into from
Closed

Correctly pad oidc tokens #70

wants to merge 2 commits into from

Conversation

hanikesn
Copy link

@hanikesn hanikesn commented Jun 5, 2018

According to the JWT spec base64 padding characters are stripped.
Fixes #65

Tested only with python 3.

hanikesn and others added 2 commits June 5, 2018 14:04
@hanikesn
Correctly pad oidc tokens
9f5b8c4 
According to the JWT spec base64 padding characters are stripped.
Fixes kubernetes-client#65
@hanikesn
Merge branch 'master' into fix-oidc-padding
6730e1e
@hanikesn hanikesn mentioned this pull request Jun 5, 2018
Closed
@bcatubig
Copy link

Hey @hanikesn ,

Would you mind updating your branch? Would love to see this get merged in.

@richerve richerve mentioned this pull request Aug 8, 2018
Closed
@richerve
Copy link

richerve commented Aug 8, 2018

@hanikesn are you still interested in updating this PR?

markuszoeller
markuszoeller reviewed Aug 16, 2018
View reviewed changes
config/kube_config_test.py
@@ -87,11 +87,11 @@ def _raise_exception(st):

TEST_OIDC_TOKEN = "test-oidc-token"
TEST_OIDC_INFO = "{\"name\": \"test\"}"
TEST_OIDC_BASE = _base64(TEST_OIDC_TOKEN) + "." + _base64(TEST_OIDC_INFO)
TEST_OIDC_BASE = _base64(TEST_OIDC_TOKEN).strip('=') + "." + _base64(TEST_OIDC_INFO).strip('=')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the length of the changed lines in this file is what makes the travis job complain. I get this output when I test the styleguide locally:

$ pycodestyle config/kube_config*
config/kube_config_test.py:90:80: E501 line too long (95 > 79 characters)
config/kube_config_test.py:94:80: E501 line too long (98 > 79 characters)

@hanikesn
Copy link
Author

I will take another look this weekend.

@iMartyn iMartyn mentioned this pull request Aug 21, 2018
Open
@iMartyn
Copy link

iMartyn commented Aug 23, 2018

Eagerly waiting for this fix to be merged so that kube-ops-view can pull it in and then we can have our monitoring working for OIDC clusters.

@joppa27 joppa27 mentioned this pull request Sep 24, 2018
Closed
@kmai
Copy link

kmai commented Jan 10, 2019

is this stale? OIDC is still broken with the k8s python client..

@danigrmartinez
Copy link

@hanikesn this fix works for me. I would love to see this merged
Thanks for the fix.

@micw523
Copy link
Contributor

micw523 commented Feb 4, 2019

Could you use autopep8 on aggressive level 2 to prepare your code? That should let you pass the Travis CI. @hanikesn

micw523
micw523 reviewed Feb 4, 2019
View reviewed changes
Copy link
Contributor

@micw523 micw523 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this PR is kind of old, let's try this...

config/kube_config_test.py
@@ -87,11 +87,11 @@ def _raise_exception(st):

TEST_OIDC_TOKEN = "test-oidc-token"
TEST_OIDC_INFO = "{\"name\": \"test\"}"
TEST_OIDC_BASE = _base64(TEST_OIDC_TOKEN) + "." + _base64(TEST_OIDC_INFO)
TEST_OIDC_BASE = _base64(TEST_OIDC_TOKEN).strip('=') + "." + _base64(TEST_OIDC_INFO).strip('=')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
TEST_OIDC_BASE = _base64(TEST_OIDC_TOKEN).strip('=') + "." + _base64(TEST_OIDC_INFO).strip('=')
TEST_OIDC_BASE = _base64(TEST_OIDC_TOKEN).strip(
'=') + "." + _base64(TEST_OIDC_INFO).strip('=')

config/kube_config_test.py
TEST_OIDC_LOGIN = TEST_OIDC_BASE + "." + TEST_CLIENT_CERT_BASE64
TEST_OIDC_TOKEN = "Bearer %s" % TEST_OIDC_LOGIN
TEST_OIDC_EXP = "{\"name\": \"test\",\"exp\": 536457600}"
TEST_OIDC_EXP_BASE = _base64(TEST_OIDC_TOKEN) + "." + _base64(TEST_OIDC_EXP)
TEST_OIDC_EXP_BASE = _base64(TEST_OIDC_TOKEN).strip('=') + "." + _base64(TEST_OIDC_EXP).strip('=')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
TEST_OIDC_EXP_BASE = _base64(TEST_OIDC_TOKEN).strip('=') + "." + _base64(TEST_OIDC_EXP).strip('=')
TEST_OIDC_EXP_BASE = _base64(TEST_OIDC_TOKEN).strip(
'=') + "." + _base64(TEST_OIDC_EXP).strip('=')

@micw523
Copy link
Contributor

micw523 commented Feb 4, 2019

/assign

@micw523
Copy link
Contributor

micw523 commented Feb 9, 2019

/check-cla

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 9, 2019
@wwade
Copy link

wwade commented Mar 13, 2019

@hanikesn @micw523 has this change been abandoned? I can create a new PR if needed and help it cross the finish line.

@micw523
Copy link
Contributor

micw523 commented Mar 13, 2019

@wwade This one was abandoned I think. The other one is still open.

@rocktavious
Copy link

@wwade @micw523 i'd really like to track the status of this as its blocking me from writing a tool with python against our OIDC backed k8s clusters. Not sure if there is a different PR to watch.

@ralphje
Copy link

ralphje commented May 1, 2019

@wwade @micw523 i'd really like to track the status of this as its blocking me from writing a tool with python against our OIDC backed k8s clusters. Not sure if there is a different PR to watch.

I guess you could also watch #79

@roycaihw
Copy link
Member

Closing in favor of #79. Thanks for the fix!

@roycaihw roycaihw closed this Jun 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@markuszoeller markuszoeller markuszoeller left review comments

@micw523 micw523 micw523 left review comments

Assignees

@micw523 micw523

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

OIDC auth uses incorrect base64 decoding