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

Missing field name in the generated template when last word of the Credential Name is greater than 7 characters #164

Closed
arunsathiya opened this issue Jan 25, 2023 · 2 comments · Fixed by #263
Assignees
Labels
op-cli Functionality to be implemented in 1Password CLI. Needs to be done by 1Password Developers.

Comments

@arunsathiya
Copy link
Contributor

op CLI version

2.12.0

Goal or desired behavior

Credential template file is generated correctly.

Current behavior

Credential template file generated is missing the field name (not credential name) when the credential name is entered to be Credentials in the setup flow:

image

func Credentials() schema.CredentialType {
	return schema.CredentialType{
		Name:          credname.Credentials,
		DocsURL:       sdk.URL("https://ngrok.com/docs/credentials"), // TODO: Replace with actual URL
		ManagementURL: sdk.URL("https://console.ngrok.com/user/security/tokens"), // TODO: Replace with actual URL
		Fields: []schema.CredentialField{
			{
				Name:                fieldname.,
				MarkdownDescription: " used to authenticate to ngrok.",
				Secret:              true,
				Composition: &schema.ValueComposition{
					Length: 43,
					Charset: schema.Charset{
						Uppercase: true,
						Lowercase: true,
						Digits:    true,
					},
				},
			},
		},
		DefaultProvisioner: provision.EnvVars(defaultEnvVarMapping),
		Importer: importer.TryAll(
			importer.TryEnvVarPair(defaultEnvVarMapping),
			TryngrokConfigFile(),
		)}
}

Notice the fieldname. part in the above code.

Relevant log output

No response

@arunsathiya arunsathiya added the op-cli Functionality to be implemented in 1Password CLI. Needs to be done by 1Password Developers. label Jan 25, 2023
@arunsathiya
Copy link
Contributor Author

I am happy to take a closer look at why this is happening only for Credentials. Will followup as soon as I do.

@arunsathiya arunsathiya changed the title Missing field name in the generated template when name is "Credentials" Missing field name in the generated template when credential name is "Credentials" Jan 25, 2023
@arunsathiya arunsathiya changed the title Missing field name in the generated template when credential name is "Credentials" Missing field name in the generated template when last word of the Credential Name is greater than 7 characters Jan 26, 2023
@arunsathiya
Copy link
Contributor Author

arunsathiya commented Jan 26, 2023

I was seeing incorrectly. It's not just Credentials that's affected. Any Credential Name with the last word greater than seven characters fails to generate the Field Name. The seven character limit comes from this part:

// As a placeholder, assume the field name is the short version (max 7 chars) of the credential name, starting
// from the last word. For example:
// "Personal Access Token" => "Token"
// "Secret Key" => "Key"
// "API Key" => "API Key"
var fieldNameSplit []string
lengthCutoff := 7
for i := range credNameSplit {
word := credNameSplit[len(credNameSplit)-1-i]
if len(strings.Join(append(fieldNameSplit, word), " ")) > lengthCutoff {
break
}
fieldNameSplit = append([]string{word}, fieldNameSplit...)
}
result.FieldName = strings.Join(fieldNameSplit, " ")
result.FieldNameUpperCamelCase = strings.Join(fieldNameSplit, "")

I think a fallback Field Name should be decided, but I am not sure what that word should be. Since Token seems used the most, should we use that? Or just set the Field Name to be the same as the Credential Name, and show a Todo that the developer should change the Field Name?

Tagging @SimonBarendse and @hculea for their insights, and anyone else that has ideas are absolutely welcome to share!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
op-cli Functionality to be implemented in 1Password CLI. Needs to be done by 1Password Developers.
Projects
None yet
1 participant