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

Flexible regex for token #370

Closed
wibeasley opened this issue Nov 15, 2021 · 5 comments
Closed

Flexible regex for token #370

wibeasley opened this issue Nov 15, 2021 · 5 comments

Comments

@wibeasley
Copy link
Member

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 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.

wibeasley referenced this issue in michalkouril/REDCapR Nov 15, 2021
@ghost
Copy link

ghost commented Nov 15, 2021 via email

@wibeasley wibeasley assigned ghost Nov 15, 2021
@wibeasley
Copy link
Member Author

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)?$"

@ghost
Copy link

ghost commented Nov 17, 2021 via email

@wibeasley
Copy link
Member Author

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 sanitize_token() is called inside 20+ functions and you don't want to the token pattern to all of them?

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.

wibeasley added a commit that referenced this issue Aug 14, 2022
@wibeasley
Copy link
Member Author

wibeasley commented Aug 14, 2022

@maeon & @michalkouril, see how you like this function and its documentation . Tell me if there's something you'd like to change.,

wibeasley added a commit that referenced this issue Aug 14, 2022
I don't know why I commented them out last night

ref #370
wibeasley added a commit that referenced this issue Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant