Skip to content

feat: add user_token_getter_url, people can get user token from app manager #686

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.

Sign up for GitHub

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

Closed
wangjingling wants to merge 1 commit into from
Closed

feat: add user_token_getter_url, people can get user token from app manager #686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 134 additions & 0 deletions cmd/auth/get_user_token_url.go.demo
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
package auth

// this file is a demo for user_token_getter_url

import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/url"
"strconv"

"github.com/cloudwego/hertz/pkg/app"
"github.com/cloudwego/hertz/pkg/common/utils"
lark "github.com/larksuite/oapi-sdk-go/v3"
larkauthen "github.com/larksuite/oapi-sdk-go/v3/service/authen/v1"
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const (
appID = ""
appSecret = ""
redirectURI
authHost = "open.feishu.cn/open.larksuite.com"
)
Comment on lines +19 to +24

@coderabbitai coderabbitai Bot Apr 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

The demo currently builds a broken authorization URL.

authHost is set to open.feishu.cn/open.larksuite.com, so this code generates https://open.feishu.cn/open.larksuite.com/open-apis/authen/v1/index instead of a valid Feishu or Lark auth endpoint. redirectURI is also empty, so even the redirect params are incomplete. As written, this demo cannot drive a successful OAuth redirect.

Also applies to: 40-50

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@cmd/auth/get_user_token_url_demo_test.go` around lines 19 - 24, The demo
constants appID, appSecret, redirectURI and authHost are incorrect/empty causing
a malformed OAuth URL; update authHost to a single valid host string (e.g.
"open.feishu.cn" or "open.larksuite.com") instead of
"open.feishu.cn/open.larksuite.com", provide a non-empty redirectURI (use a
valid local/test redirect like "https://example.com/callback" or a clearly
marked placeholder), and ensure appID/appSecret are set to realistic demo
placeholders so the URL builder used in the test (referenced by the constants
appID, appSecret, redirectURI, authHost) produces a correct auth endpoint; apply
the same fixes for the repeated constants in the later block referenced (lines
~40-50).


var (
larkClient = lark.NewClient(appID, appSecret)
)

// UserAuth handles the initial step of the OAuth flow by redirecting the user to the Lark authorization page.
func UserAuth(ctx context.Context, c *app.RequestContext) {
state := c.Query("state")
if state == "" {
c.JSON(http.StatusBadRequest, utils.H{"error": "missing state"})
return
}

scope := c.Query("scope")

authURLObj, _ := url.Parse(fmt.Sprintf("https://%s/open-apis/authen/v1/index", authHost))
query := authURLObj.Query()
query.Set("redirect_uri", redirectURI)
query.Set("app_id", appID)
query.Set("state", state)
if scope != "" {
query.Set("scope", scope)
}
authURLObj.RawQuery = query.Encode()

c.Redirect(http.StatusFound, []byte(authURLObj.String()))
}

// OAuthCallback processes the OAuth callback from Lark, fetches the user access token, and sends it back to the local server.
func OAuthCallback(ctx context.Context, c *app.RequestContext) {
code := c.Query("code")
state := c.Query("state")

if code == "" {
c.JSON(http.StatusBadRequest, utils.H{"error": "missing code"})
return
}

stateInt, err := strconv.Atoi(state)
if err != nil {
c.JSON(http.StatusBadRequest, utils.H{"error": "invalid state parameter, must be integer"})
return
Comment on lines +63 to +66

@coderabbitai coderabbitai Bot Apr 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Keep the callback port separate from OAuth state.

Here state is only validated as an integer and then reused as the localhost port that receives the token POST. That drops the normal request/response binding that state is supposed to provide and lets the caller choose which local service receives the token. Use an independent random state for CSRF protection and carry the callback port in a separate signed/opaque value.

Also applies to: 102-131

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@cmd/auth/get_user_token_url_demo_test.go` around lines 63 - 66, The code is
incorrectly treating the OAuth `state` as the localhost callback port (state ->
stateInt) which breaks CSRF semantics; instead generate and validate `state` as
a random CSRF token and move the callback port into a separate signed/opaque
value (e.g., callbackToken or callbackInfo) that you encode/verify server-side.
Change the flow in the functions handling the GET and POST (the places that
parse `state` via strconv.Atoi and use stateInt as a port) to: 1) expect `state`
to be a random string and only validate it against the stored/expected CSRF
value, 2) read the callback port from a distinct parameter (or from a signed
payload) and verify its signature/expiry before using it to bind the local
listener, and 3) replace the existing error branch that returns "invalid state
parameter, must be integer" with proper CSRF/state validation errors while
moving any port-parsing (Atoi) to the separate callback param handling code.

}

// get user_access_token
req := larkauthen.NewCreateAccessTokenReqBuilder().
Body(larkauthen.NewCreateAccessTokenReqBodyBuilder().
GrantType("authorization_code").
Code(code).
Build()).
Build()

resp, err := larkClient.Authen.AccessToken.Create(ctx, req)
if err != nil {
c.JSON(http.StatusInternalServerError, utils.H{"error": "failed to get access token", "detail": err.Error()})
return
}

if !resp.Success() {
c.JSON(http.StatusInternalServerError, utils.H{"error": "feishu API returned error", "code": resp.Code, "msg": resp.Msg})
return
}

data := resp.Data
if data == nil || data.AccessToken == nil {
c.JSON(http.StatusInternalServerError, utils.H{"error": "empty data in response"})
return
}

// TODO check user permission or scope

dataBytes, err := json.Marshal(data)
if err != nil {
c.JSON(http.StatusInternalServerError, utils.H{"error": "failed to marshal data"})
return
}

html := fmt.Sprintf(`<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<title>Lark Token</title>
</head>
<body>
<p>Sending token...</p>
<script>
window.onload = function() {
var url = 'http://127.0.0.1:%d/user_access_token';
var data = %s;
fetch(url, {
method: 'POST',
headers: {
'Content-Type': 'application/json'
},
body: JSON.stringify(data)
}).then(function(response) {
document.body.innerHTML += '<p>Token sended，close in 3 seconds...</p>';
setTimeout(function() {
window.close();
}, 3000);
}).catch(function(err) {
document.body.innerHTML += '<p>Token send fail: ' + err + '</p>';
});
};
</script>
</body>
</html>`, stateInt, string(dataBytes))
Comment on lines +102 to +131

@coderabbitai coderabbitai Bot Apr 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Embedding marshaled JSON directly into a <script> body is unsafe.

json.Marshal does not escape <, >, or / by default, so any field in data (e.g. an error/msg string round-tripped through the SDK, or a future scope/extra field) that contains </script> will break out of the script context and become live HTML. The token JSON itself is unlikely to contain that today, but this is a footgun if data ever grows. Use json.Encoder.SetEscapeHTML(true) (the default) and additionally guard against </:

🛡️ Safer embedding 
-	dataBytes, err := json.Marshal(data)
+	var buf bytes.Buffer
+	enc := json.NewEncoder(&buf)
+	enc.SetEscapeHTML(true)
+	if err := enc.Encode(data); err != nil {
+		c.JSON(http.StatusInternalServerError, utils.H{"error": "failed to marshal data"})
+		return
+	}
+	// Defensively neutralize any "</" sequences before injecting into <script>.
+	dataBytes := bytes.ReplaceAll(bytes.TrimRight(buf.Bytes(), "\n"), []byte("</"), []byte("<\\/"))

(Add "bytes" to the imports.)

Also, line 121 has a small typo: "Token sended，close in 3 seconds..." — please change to "Token sent, close in 3 seconds..." and replace the full‑width comma ， with an ASCII , to keep the UI string consistent.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@cmd/auth/get_user_token_url_demo_test.go` around lines 102 - 131, The HTML
template embeds JSON unsafely via string(dataBytes) into the script (variable
html); fix by re-encoding dataBytes with a json.Encoder that has
SetEscapeHTML(true) into a bytes.Buffer (or re-marshal via an Encoder) and then
replace any literal "</" with "<\/" before interpolating into the template to
prevent breaking out of the script context; use that escaped JSON in place of
string(dataBytes). Also correct the UI string "Token sended，close in 3
seconds..." to "Token sent, close in 3 seconds..." (replace the full‑width comma
with an ASCII comma).


c.Data(http.StatusOK, "text/html; charset=utf-8", []byte(html))
}
Loading
Loading