Skip to content
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

Fix tokenless authentication chart #141

Merged
merged 1 commit into from
Mar 21, 2018

Conversation

pluehne
Copy link
Contributor

@pluehne pluehne commented Mar 21, 2018

The format of the gitauth.log file changed from GitHub Enterprise 2.11 to 2.12. This makes it necessary to adjust the regular expression responsible for fetching user names and repositories.

Unfortunately, it seems that we can no longer rely on the order of the fields in gitauth.log. For this reason, multiple grep iterations are performed and the regular expression is changed to use look-aheads to allow for arbitrary field orders.

For backward compatibility with GitHub Enterprise 2.11 and earlier, a switch is implemented that executes the old query if necessary.

Addresses the second issue mentioned in #139.

@pluehne
Copy link
Contributor Author

pluehne commented Mar 21, 2018

@larsxschneider: Please have a look at my changes. I hope that you’re okay with the look-ahead solution. What it does is capture the member and path fields, but with a look-ahead, which means that the cursor position is not altered, which is why the fields are matched in arbitrary orders.

I’m still thinking whether we could simply support GitHub Enterprise 2.11 by extending the status field to match either 200 or OK and the hashed_token field to match either nil or to be missing entirely. What do you think?

@pluehne pluehne force-pushed the patrick/fix-tokenless-auth-chart branch 2 times, most recently from e099d98 to 75d12e3 Compare March 21, 2018 12:32
The format of the gitauth.log file changed from GitHub Enterprise 2.11
to 2.12. This makes it necessary to adjust the regular expression
responsible for fetching user names and repositories.

Unfortunately, it seems that we can no longer rely on the order of the
fields in gitauth.log. For this reason, multiple grep iterations are
performed and the regular expression is changed to use look-aheads to
allow for arbitrary field orders.

For backward compatibility with GitHub Enterprise 2.11 and earlier, a
switch is implemented that executes the old query if necessary.
@pluehne pluehne force-pushed the patrick/fix-tokenless-auth-chart branch from 75d12e3 to ecfddb4 Compare March 21, 2018 12:50
@pluehne
Copy link
Contributor Author

pluehne commented Mar 21, 2018

@larsxschneider: I implemented the backward compatibility check, and this pull request is now ready for review 😄.

Copy link
Collaborator

@larsxschneider larsxschneider left a comment

Choose a reason for hiding this comment

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

Nice! Thank you 👍

@larsxschneider larsxschneider merged commit a7db590 into master Mar 21, 2018
@larsxschneider larsxschneider deleted the patrick/fix-tokenless-auth-chart branch March 21, 2018 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants