update PAT validation for new token formats

r-lib / gh

Minimalistic GitHub API client in R

https://gh.r-lib.org

Other

223 stars 52 forks source link

Closed fmichonneau closed 3 years ago

fmichonneau commented 3 years ago

fix #148

update the regular expression to match the new token formats.

Do we want to allow for the non-PAT prefixes? in other words, should only ghp_ prefixes be allowed? (this PR allows the 5 prefixes mentioned in the blog post
Should tests be modified to account for this regular expression change?
Do we want to already allow for up to 255 characters?

jennybc commented 3 years ago

I think it makes sense to accept the non-PAT prefixes
I would accept up to 255 characters in the regular expression that has the prefixes. Intent: I hope this means "tokens created under the new regime".
I would leave the exact length requirement of 40 for the regular expression targeting "legacy" PATs. This has been very useful for catching people providing a regular password as PAT.
Seems like we should add a test with a new-style PAT to make sure it does NOT error. I just got a new PAT and it does have the new form.

fmichonneau commented 3 years ago

let me know what you think of these new changes. Thanks!

jennybc commented 3 years ago

Thanks!

fmichonneau commented 3 years ago

Thank you!