-
Notifications
You must be signed in to change notification settings - Fork 48
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
Flexible regex for token #370
Comments
Great! I like the token_pattern as a parameter approach. My token format is variable length base64 encoded -- so slightly more complicated but absolutely doable.
I'll just need the parameter specified once instead of with each function call -- such as env variable, global option, etc.
… On Nov 14, 2021, at 9:07 PM, Will Beasley ***@***.***> wrote:
Spun off from the comments in ***@***.*** <michalkouril@9143b85>
@maeon <https://github.com/maeon>, cool. I like it. I have a similar approach where LDAP/AD credentials are traded for REDCap tokens <https://ouhscbbmc.github.io/REDCapR/articles/SecurityDatabase.html>. But it sounds like your approach bundles it more seamlessly.
Your token format is consistent, correct? Instead of turning off the check, would it work with your approach to allow the user to specify the regex?
We could add a token_pattern parameter with a default of "^([0-9A-Fa-f]{32})(?:\\n)?$". In the code executed in AWS, your user could pass something like token_pattern = "^([0-9A-Fa-f]{128})(?:\\n)?$".
To people joining this thread, this won't weaken security because this regex has no role in authentication. It simply notifies the user of a if their token doesn't look right, and therefore hopefully helps them diagnose the problem quicker.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#370>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACDOLZOPHDXLZUXNYZ7SWDDUMBTPTANCNFSM5IASMRVA>.
Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Another option is the regex pattern could accept both types of tokens. Does anyone have a preference? To accept both types of tokens, the parameter default would be something like (I'm guessing on the length). token_pattern = "^([0-9A-Fa-f]{32}|[0-9A-Za-z]{20})(?:\\n)?$" |
I believe it make sense to default to the current REDCap token pattern and let user specify a custom one if the situation warrants it.
… On Nov 15, 2021, at 1:42 AM, Will Beasley ***@***.***> wrote:
Another option is the regex pattern could accept both types of tokens. Does anyone have a preference?
To accept both types of tokens, the parameter default would be something like (I'm guessing on the length).
token_pattern = "^([0-9A-Fa-f]{32}|[0-9A-Za-z]{20})(?:\\n)?$"
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub <#370 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACDOLZPSN3BR5IYGU5C2MEDUMCTWBANCNFSM5IASMRVA>.
Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Cool. I've rearranged your code a little. I don't use environmental variables, so please write it so it works most naturally for you. If you like it, please submit a PR. pattern <-
if (Sys.getenv("REDCAP_TOKEN_PATTERN") != "") {
Sys.getenv("REDCAP_TOKEN_PATTERN")
} else {
"^([0-9A-Fa-f]{32})(?:\\n)?$"
} Also, let me make sure I understand, "I'll just need the parameter specified once instead of with each function call -- such as env variable, global option, etc." Is it because If so, one option would be for me to expand the project object to include all the methods (not just read & write). Or maybe that's what you meant by "global option". I haven't kept up with adding methods b/c no one seems to use the project object and I didn't want to duplicate all the tests unnecessarily. But if the project object was used, the token pattern would be passed once initially and stored internally, then passed each time to a method. |
@maeon & @michalkouril, see how you like this function and its documentation . Tell me if there's something you'd like to change., |
I don't know why I commented them out last night ref #370
Spun off from the comments in michalkouril@9143b85
@maeon, cool. I like it. I have a similar approach where LDAP/AD credentials are traded for REDCap tokens. But it sounds like your approach bundles it more seamlessly.
Your token format is consistent, correct? Instead of turning off the check, would it work with your approach to allow the user to specify the regex?
We could add a
token_pattern
parameter with a default of"^([0-9A-Fa-f]{32})(?:\\n)?$"
. In the code executed in AWS, your user could pass something liketoken_pattern = "^([0-9A-Fa-f]{128})(?:\\n)?$"
.To people joining this thread, this won't weaken security because this regex has no role in authentication. It simply notifies the user of a if their token doesn't look right, and therefore hopefully helps them diagnose the problem quicker.
The text was updated successfully, but these errors were encountered: