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

require whisk auth fix #480

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

steven0711dong
Copy link
Contributor

No description provided.

@steven0711dong steven0711dong force-pushed the requirewhiskauthformatfix branch from 92e7523 to 1de1d9c Compare April 8, 2020 17:30
Copy link
Member

@rabbah rabbah left a comment

Choose a reason for hiding this comment

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

I apologize no one reviewed this sooner. I added a comment about generating alphanumeric secrets instead of integer only. Also this needs either some go unit tests or integration (scala) tests.

@@ -849,49 +854,76 @@ func getWebSecureAnnotations(action *whisk.Action) whisk.KeyValueArr {
return webKvArr[0:j]
}

func getNewSecret(secret interface{}) string {
_, isJSONNum := secret.(json.Number)
Copy link
Member

Choose a reason for hiding this comment

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

I'd replace this with the ability to generate an alphanumeric secret instead.
Something like:

rand.Seed(time.Now().UnixNano())
var alphabet = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")
b := make([]rune, SECRET_LENGTH)
for i := range b {
   b[i] = alphabet[rand.Intn(len(alphabet))]
}
return string(b)

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

Successfully merging this pull request may close these issues.

2 participants