Create new connector plugin to VDSaccount#312
Create new connector plugin to VDSaccount#312mike-brown8 wants to merge 16 commits intoapache:mainfrom
Conversation
创建 connector-vds/vds.go
|
Links in go.mod and vds.go may need to be modified. I can't change it on my mobile phone, sorry. |
There was a problem hiding this comment.
Pull request overview
Adds a new Apache Answer connector plugin that integrates with VDS Account OAuth to enable SSO login via VDS, including embedded assets and i18n resources.
Changes:
- Introduces a new
connector-vdsGo module implementing the OAuth authorize/token/userinfo flow plus platform-signature management. - Adds connector metadata (
info.yaml), i18n strings (en/zh), and end-user documentation (README.md). - Embeds a connector logo asset and ships an Apache 2.0 license file for the plugin directory.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| connector-vds/vds.go | Core connector implementation (OAuth flow, signature cache/refresh, logo embedding, config UI). |
| connector-vds/constants.go | Hardcoded VDS endpoints, JSON paths, and username constraints/constants. |
| connector-vds/go.mod | New module definition and dependencies for the connector. |
| connector-vds/go.sum | Dependency checksums for the connector module. |
| connector-vds/info.yaml | Plugin metadata (slug/type/version/author/link). |
| connector-vds/i18n/translation.go | Translation key constants for plugin UI strings. |
| connector-vds/i18n/en_US.yaml | English translations for plugin name/description/config fields. |
| connector-vds/i18n/zh_CN.yaml | Chinese translations for plugin name/description/config fields. |
| connector-vds/README.md | Installation/configuration docs and changelog for the connector. |
| connector-vds/LICENSE | Apache 2.0 license text included with the plugin. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func randomString(length int) string { | ||
| const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_" | ||
| chars := []rune(charset) | ||
| result := make([]rune, length) | ||
| for i := range result { | ||
| result[i] = chars[rand.Intn(len(chars))] | ||
| } | ||
| return string(result) | ||
| } |
There was a problem hiding this comment.
randomString uses math/rand (not cryptographically secure, and not seeded here), but it is used for generating the OAuth state value. For CSRF protection the state should be generated with crypto/rand (or a shared token helper) and then validated on callback; otherwise the state is predictable and provides no protection.
| func (g *Connector) ConnectorSender(ctx *plugin.GinContext, receiverURL string) (redirectURL string) { | ||
| oauth2Config := &oauth2.Config{ | ||
| ClientID: g.Config.ClientID, | ||
| ClientSecret: g.Config.ClientSecret, | ||
| Endpoint: oauth2.Endpoint{ | ||
| AuthURL: AuthorizeURL, | ||
| TokenURL: TokenURL, | ||
| }, | ||
| RedirectURL: receiverURL, | ||
| Scopes: strings.Split(DefaultScope, ","), | ||
| } | ||
| state := randomString(24) | ||
| return oauth2Config.AuthCodeURL(state) | ||
| } |
There was a problem hiding this comment.
ConnectorSender generates an OAuth state but ConnectorReceiver never validates it (and there’s no persistence of the expected value). This defeats the CSRF protection that state is intended to provide. Consider storing the generated state in a short-lived cache keyed to the user/session (see user-center-slack which caches and validates oauth_state_*) and verify ctx.Query("state") in ConnectorReceiver.
| // Normalize username: replace invalid characters with underscore | ||
| username := userInfo.Username | ||
| for i, r := range username { | ||
| if !((r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '.' || r == '_' || r == '-') { | ||
| username = string([]rune(username[:i])) + "_" + string([]rune(username[i+1:])) | ||
| } | ||
| } |
There was a problem hiding this comment.
This username normalization mutates username while iterating over it with for i, r := range username. Because i is a byte index into the original string, and the code slices username[:i] / username[i+1:] after modifications, it can skip characters or corrupt multi-byte UTF-8 sequences. Prefer operating on a []rune copy (replace invalid runes in-place) or reuse the existing Answer helper used in connector-basic (checker.IsInvalidUsername + a regexp replacement).
| jsonData, err := json.Marshal(payload) | ||
| if err != nil { | ||
| return "", time.Time{}, fmt.Errorf("failed to marshal signature request: %w", err) | ||
| } | ||
|
|
||
| req, err := http.NewRequest("POST", SignatureURL, strings.NewReader(string(jsonData))) | ||
| if err != nil { | ||
| return "", time.Time{}, fmt.Errorf("failed to create signature request: %w", err) |
There was a problem hiding this comment.
requestSignature marshals JSON to []byte and then converts it to string just to create a strings.NewReader. This adds an unnecessary allocation/copy. Prefer bytes.NewReader(jsonData) (or bytes.NewBuffer(jsonData)) to send the marshaled bytes directly.
| } | ||
|
|
||
| ctx := context.WithValue(context.Background(), oauth2.HTTPClient, httpClient) | ||
| token, err := oauth2Config.Exchange(ctx, code) | ||
| if err != nil { |
There was a problem hiding this comment.
This uses a fresh context.Background() rather than the incoming request context, so token exchange won’t be cancelled if the client disconnects or the request times out upstream. Prefer deriving from ctx.Request.Context() (or passing a context down from ConnectorReceiver) when calling oauth2Config.Exchange.
| } | ||
|
|
||
| func (g *Connector) ConnectorLogoSVG() string { | ||
| return g.getLogoDataURICached() |
There was a problem hiding this comment.
ConnectorLogoSVG currently returns a PNG data URI (data:image/png;base64,...). In this repo, connectors typically return either raw SVG content or a base64-encoded SVG string (without a data: prefix) from ConnectorLogoSVG (see connector-github/connector-dingtalk). Returning a PNG data URI may not render correctly if the UI assumes SVG/base64. Consider embedding an SVG asset (or converting the PNG to SVG) and returning it in the expected format, or confirm the UI supports data:image/png here.
| return g.getLogoDataURICached() | |
| // Return raw SVG content as expected by the UI and other connectors. | |
| return `<svg xmlns="http://www.w3.org/2000/svg" width="120" height="32" viewBox="0 0 120 32"> | |
| <defs> | |
| <linearGradient id="vdsGradient" x1="0%" y1="0%" x2="100%" y2="0%"> | |
| <stop offset="0%" stop-color="#0052CC"/> | |
| <stop offset="100%" stop-color="#2684FF"/> | |
| </linearGradient> | |
| </defs> | |
| <rect x="1" y="1" width="118" height="30" rx="6" ry="6" fill="url(#vdsGradient)" /> | |
| <text x="50%" y="50%" dominant-baseline="middle" text-anchor="middle" | |
| font-family="system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif" | |
| font-size="16" font-weight="600" fill="#FFFFFF"> | |
| VDS | |
| </text> | |
| </svg>` |
| func (g *Connector) ConfigFields() []plugin.ConfigField { | ||
| return []plugin.ConfigField{ | ||
| { | ||
| Name: "client_id", | ||
| Type: plugin.ConfigTypeInput, | ||
| Title: plugin.MakeTranslator(i18n.ConfigClientIDTitle), | ||
| Required: true, | ||
| UIOptions: plugin.ConfigFieldUIOptions{ | ||
| InputType: plugin.InputTypeText, | ||
| }, | ||
| Value: g.Config.ClientID, | ||
| }, | ||
| { | ||
| Name: "client_secret", | ||
| Type: plugin.ConfigTypeInput, | ||
| Title: plugin.MakeTranslator(i18n.ConfigClientSecretTitle), | ||
| Required: true, | ||
| UIOptions: plugin.ConfigFieldUIOptions{ | ||
| InputType: plugin.InputTypePassword, | ||
| }, | ||
| Value: g.Config.ClientSecret, | ||
| }, | ||
| } |
There was a problem hiding this comment.
The i18n files define description strings for these config fields, but the ConfigField entries don’t set Description, so the UI won’t show the help text. Add Description: plugin.MakeTranslator(i18n.ConfigClientIDDescription) / ConfigClientSecretDescription to match the pattern used in connector-basic.
| ### v1.1.0 (2026-03-31) | ||
| - 🎯 **大幅简化配置** - 从15个字段简化到仅4个必需字段 | ||
| - 🔒 **硬编码VDS端点** - 确保始终连接到正确的VDS服务器 | ||
| - 🛡️ **移除邮箱验证检查** - 简化用户信息处理流程 | ||
| - 🎨 **改进Logo处理** - 支持PNG base64和SVG格式 | ||
| - 📝 **优化代码结构** - 提高代码可维护性和清晰度 |
There was a problem hiding this comment.
The changelog claims the config was simplified to “4 required fields”, but the current implementation/config UI only exposes 2 required fields (client_id and client_secret). Please update the changelog to reflect the actual behavior so users aren’t confused during setup/upgrades.
Code partly written by AI.