mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
## Problem The OAuth2 application settings form rejects application names with spaces and callbacks using native application schemes. A VS Code extension client registered through DCR exposed this: `VS Code Coder Extension` and `vscode://coder.coder-remote/oauth/callback` were already stored, but the admin form prevented editing them. ## Fix - Allow application names with spaces and punctuation, limited to 64 UTF-8 bytes on both client and server. - Allow native callback schemes while rejecting dangerous schemes, unsupported URNs, opaque callbacks such as `localhost:3000`, and empty targets such as `vscode:`. HTTP(S) callbacks still require a host. - Reuse the public-client registration policy when admins update public clients, so `mailto`, `tel`, `sms`, external HTTP, and fragments remain rejected. - Trim surrounding whitespace from names and callbacks when submitting, without stripping spaces while typing. - Update the admin API request validators too, so accepted form values can be saved. The server remains authoritative for URL parsing. Public-client edits reuse the existing DCR redirect validator using the stored client type; confidential admin-created applications retain their existing HTTP policy. Native host/path callbacks and the supported OOB URN remain accepted. ## Validation - Vitest component tests assert create/edit submission payloads, whitespace normalization, Unicode byte limits, and rejection of unsafe or malformed callbacks. - Storybook stories cover accepted native-client values and invalid callback states. - Go tests cover name boundaries and accepted/rejected callback values through both admin create and update requests. - `make pre-commit` and targeted tests run before pushing; CI checked on each revision. <details> <summary>Implementation decisions</summary> - Replace the shared slug validator only for OAuth2 application names; leave user, workspace, and template names unchanged. - Share the backend's 64-byte name limit with the frontend through the generated `OAuth2AppNameMaxBytes` constant. - Reuse `codersdk.ValidateRedirectURIScheme` for common admin callback validation. Public-client updates additionally call `codersdk.ValidateRedirectURIs`; confidential admin HTTP behavior is preserved. - Normalize name and callback whitespace in the submitted payload rather than on each keystroke. - Keep behavior assertions in Vitest and use Storybook for visual states. </details> Closes ENG-3342 > 🤖 This PR was created with the help of Coder Agents, and needs a human review. 🧑💻
153 lines
4.6 KiB
Go
153 lines
4.6 KiB
Go
package codersdk
|
|
|
|
import (
|
|
"fmt"
|
|
"regexp"
|
|
"strings"
|
|
|
|
"golang.org/x/xerrors"
|
|
|
|
"github.com/coder/coder/v2/coderd/util/namesgenerator"
|
|
)
|
|
|
|
var (
|
|
UsernameValidRegex = regexp.MustCompile("^[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*$")
|
|
usernameReplace = regexp.MustCompile("[^a-zA-Z0-9-]*")
|
|
|
|
templateVersionName = regexp.MustCompile(`^[a-zA-Z0-9]+(?:[_.-]{1}[a-zA-Z0-9]+)*$`)
|
|
templateDisplayName = regexp.MustCompile(`^[^\s](.*[^\s])?$`)
|
|
)
|
|
|
|
// UsernameFrom returns a best-effort username from the provided string.
|
|
//
|
|
// It first attempts to validate the incoming string, which will
|
|
// be returned if it is valid. It then will attempt to extract
|
|
// the username from an email address. If no success happens during
|
|
// these steps, a random username will be returned.
|
|
func UsernameFrom(str string) string {
|
|
if valid := NameValid(str); valid == nil {
|
|
return str
|
|
}
|
|
emailAt := strings.LastIndex(str, "@")
|
|
if emailAt >= 0 {
|
|
str = str[:emailAt]
|
|
}
|
|
str = usernameReplace.ReplaceAllString(str, "")
|
|
if valid := NameValid(str); valid == nil {
|
|
return str
|
|
}
|
|
return namesgenerator.NameDigitWith("-")
|
|
}
|
|
|
|
// NameValid returns whether the input string is a valid name.
|
|
// It is a generic validator for any name (user, workspace, template, role name, etc.).
|
|
func NameValid(str string) error {
|
|
if len(str) > 32 {
|
|
return xerrors.New("must be <= 32 characters")
|
|
}
|
|
if len(str) < 1 {
|
|
return xerrors.New("must be >= 1 character")
|
|
}
|
|
// Avoid conflicts with routes like /templates/new and /groups/create.
|
|
if str == "new" || str == "create" {
|
|
return xerrors.Errorf("cannot use %q as a name", str)
|
|
}
|
|
matched := UsernameValidRegex.MatchString(str)
|
|
if !matched {
|
|
return xerrors.New("must be alphanumeric with hyphens")
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// OAuth2AppNameMaxBytes is the maximum UTF-8 byte length of an OAuth2
|
|
// application name.
|
|
const OAuth2AppNameMaxBytes = 64
|
|
|
|
// OAuth2AppNameValid returns whether the input string is a valid OAuth2
|
|
// application name: a non-empty string of at most OAuth2AppNameMaxBytes UTF-8
|
|
// bytes with no leading or trailing whitespace. Unlike NameValid, it allows
|
|
// spaces and other characters that appear in dynamically registered client
|
|
// names such as "VS Code Coder Extension".
|
|
func OAuth2AppNameValid(str string) error {
|
|
if len(str) < 1 {
|
|
return xerrors.New("must be >= 1 character")
|
|
}
|
|
if len(str) > OAuth2AppNameMaxBytes {
|
|
return xerrors.Errorf("must be <= %d bytes", OAuth2AppNameMaxBytes)
|
|
}
|
|
if strings.TrimSpace(str) != str {
|
|
return xerrors.New("must not have leading or trailing whitespace")
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// TemplateVersionNameValid returns whether the input string is a valid template version name.
|
|
func TemplateVersionNameValid(str string) error {
|
|
if len(str) > 64 {
|
|
return xerrors.New("must be <= 64 characters")
|
|
}
|
|
matched := templateVersionName.MatchString(str)
|
|
if !matched {
|
|
return xerrors.New("must be alphanumeric with underscores and dots")
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// DisplayNameValid returns whether the input string is a valid template display name.
|
|
func DisplayNameValid(str string) error {
|
|
if len(str) == 0 {
|
|
return nil // empty display_name is correct
|
|
}
|
|
if len(str) > 64 {
|
|
return xerrors.New("must be <= 64 characters")
|
|
}
|
|
matched := templateDisplayName.MatchString(str)
|
|
if !matched {
|
|
return xerrors.New("must be alphanumeric with spaces")
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// UserRealNameValid returns whether the input string is a valid real user name.
|
|
func UserRealNameValid(str string) error {
|
|
if len(str) > 128 {
|
|
return xerrors.New("must be <= 128 characters")
|
|
}
|
|
|
|
if strings.TrimSpace(str) != str {
|
|
return xerrors.New("must not have leading or trailing whitespace")
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// GroupNameValid returns whether the input string is a valid group name.
|
|
func GroupNameValid(str string) error {
|
|
// We want to support longer names for groups to allow users to sync their
|
|
// group names with their identity providers without manual mapping. Related
|
|
// to: https://github.com/coder/coder/issues/15184
|
|
limit := 255
|
|
if len(str) > limit {
|
|
return xerrors.New(fmt.Sprintf("must be <= %d characters", limit))
|
|
}
|
|
// Avoid conflicts with routes like /groups/new and /groups/create.
|
|
if str == "new" || str == "create" {
|
|
return xerrors.Errorf("cannot use %q as a name", str)
|
|
}
|
|
matched := UsernameValidRegex.MatchString(str)
|
|
if !matched {
|
|
return xerrors.New("must be alphanumeric with hyphens")
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// NormalizeUserRealName normalizes a user name such that it will pass
|
|
// validation by UserRealNameValid. This is done to avoid blocking
|
|
// little Bobby Whitespace from using Coder.
|
|
func NormalizeRealUsername(str string) string {
|
|
s := strings.TrimSpace(str)
|
|
if len(s) > 128 {
|
|
s = s[:128]
|
|
}
|
|
return s
|
|
}
|