Skip to content

feat: add i18n in login page #5947

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

Merged
merged 8 commits into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
2 changes: 2 additions & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@
ci/helm-chart/ @Matthew-Beckett @alexgorbatchev

docs/install.md @GNUxeava

src/node/i18n/locales/zh-cn.json @zhaozhiming
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to self: may have to give write access...

Related: https://github.com/orgs/community/discussions/23042

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know much about .github/CODEOWNERS. Is there anything else I need to do?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, you're good here!

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
"express": "5.0.0-alpha.8",
"http-proxy": "^1.18.0",
"httpolyglot": "^0.1.2",
"i18next": "^22.4.6",
"js-yaml": "^4.0.0",
"limiter": "^1.1.5",
"pem": "^1.14.2",
Expand Down
8 changes: 4 additions & 4 deletions src/browser/pages/login.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
http-equiv="Content-Security-Policy"
content="style-src 'self'; script-src 'self' 'unsafe-inline'; manifest-src 'self'; img-src 'self' data:; font-src 'self' data:;"
/>
<title>{{APP_NAME}} login</title>
<title>{{I18N_LOGIN_TITLE}}</title>
<link rel="icon" href="{{CS_STATIC_BASE}}/src/browser/media/favicon-dark-support.svg" />
<link rel="alternate icon" href="{{CS_STATIC_BASE}}/src/browser/media/favicon.ico" />
<link rel="manifest" href="{{BASE}}/manifest.json" crossorigin="use-credentials" />
Expand All @@ -25,7 +25,7 @@
<div class="card-box">
<div class="header">
<h1 class="main">{{WELCOME_TEXT}}</h1>
<div class="sub">Please log in below. {{PASSWORD_MSG}}</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Part of me thinks we should keep the " " here between {{I18N_LOGIN_BELOW}}{{PASSWORD_MSG}} since I could see someone forgetting to add it to the locales file.

<div class="sub">{{I18N_LOGIN_BELOW}} {{PASSWORD_MSG}}</div>
</div>
<div class="content">
<form class="login-form" method="post">
Expand All @@ -38,11 +38,11 @@ <h1 class="main">{{WELCOME_TEXT}}</h1>
autofocus
class="password"
type="password"
placeholder="PASSWORD"
placeholder="{{I18N_PASSWORD_PLACEHOLDER}}"
name="password"
autocomplete="current-password"
/>
<input class="submit -button" value="SUBMIT" type="submit" />
<input class="submit -button" value="{{I18N_SUBMIT}}" type="submit" />
</div>
{{ERROR}}
</form>
Expand Down
8 changes: 8 additions & 0 deletions src/node/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export interface UserProvidedArgs extends UserProvidedCodeArgs {
verbose?: boolean
"app-name"?: string
"welcome-text"?: string
lng?: string
/* Positional arguments. */
_?: string[]
}
Expand Down Expand Up @@ -264,6 +265,13 @@ export const options: Options<Required<UserProvidedArgs>> = {
`,
deprecated: true,
},
lng: {
type: "string",
description: `
Language show on login page, more infomations to read up on
https://en.wikipedia.org/wiki/IETF_language_tag.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Language show on login page, more infomations to read up on
https://en.wikipedia.org/wiki/IETF_language_tag.
Language to show on the login page, more info see
https://en.wikipedia.org/wiki/IETF_language_tag

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had put the description to the --locale flag.

`,
},
}

export const optionDescriptions = (opts: Partial<Options<Required<UserProvidedArgs>>> = options): string[] => {
Expand Down
21 changes: 21 additions & 0 deletions src/node/i18n/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import i18next, { init } from "i18next"
import * as en from "./locales/en.json"
import * as zhCn from "./locales/zh-cn.json"

init({
lng: "en",
fallbackLng: "en", // language to use if translations in user language are not available.
returnNull: false,
lowerCaseLng: true,
debug: process.env.NODE_ENV === "development",
resources: {
en: {
translation: en,
},
"zh-cn": {
translation: zhCn,
},
},
})

export default i18next
13 changes: 13 additions & 0 deletions src/node/i18n/locales/en.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"LOGIN_TITLE": "{{app}} login",
"LOGIN_BELOW": "Please log in below.",
"WELCOME": "Welcome to {{app}}",
"LOGIN_PASSWORD": "Check the config file at {{configFile}} for the password.",
"LOGIN_USING_ENV_PASSWORD": "Password was set from $PASSWORD.",
"LOGIN_USING_HASHED_PASSWORD": "Password was set from $HASHED_PASSWORD.",
"SUBMIT": "SUBMIT",
"PASSWORD_PLACEHOLDER": "PASSWORD",
"LOGIN_RATE_LIMIT": "Login rate limited!",
"MISS_PASSWORD": "Missing password",
"INCORRECT_PASSWORD": "Incorrect password"
}
13 changes: 13 additions & 0 deletions src/node/i18n/locales/zh-cn.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
Copy link
Member

@code-asher code-asher Jan 5, 2023

Choose a reason for hiding this comment

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

Would you be willing to add yourself as a code owner for this file in .github/CODEOWNERS in case we add new strings in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. It's my pleasure.

"LOGIN_TITLE": "{{app}} 登录",
"LOGIN_BELOW": "请在下面登录。",
"WELCOME": "欢迎来到 {{app}}",
"LOGIN_PASSWORD": "查看配置文件 {{configFile}} 中的密码。",
"LOGIN_USING_ENV_PASSWORD": "密码在 $PASSWORD 中设置。",
"LOGIN_USING_HASHED_PASSWORD": "密码在 $HASHED_PASSWORD 中设置。",
"SUBMIT": "提交",
"PASSWORD_PLACEHOLDER": "密码",
"LOGIN_RATE_LIMIT": "登录速率限制!",
"MISS_PASSWORD": "缺少密码",
"INCORRECT_PASSWORD": "密码不正确"
}
22 changes: 14 additions & 8 deletions src/node/routes/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { CookieKeys } from "../../common/http"
import { rootPath } from "../constants"
import { authenticated, getCookieOptions, redirect, replaceTemplates } from "../http"
import { getPasswordMethod, handlePasswordValidation, humanPath, sanitizeString, escapeHtml } from "../util"
import i18n from "../i18n"

// RateLimiter wraps around the limiter library for logins.
// It allows 2 logins every minute plus 12 logins every hour.
Expand All @@ -28,21 +29,26 @@ export class RateLimiter {

const getRoot = async (req: Request, error?: Error): Promise<string> => {
const content = await fs.readFile(path.join(rootPath, "src/browser/pages/login.html"), "utf8")
const lng = req.args["lng"] || "en"
i18n.changeLanguage(lng)
const appName = req.args["app-name"] || "code-server"
const welcomeText = req.args["welcome-text"] || `Welcome to ${appName}`
let passwordMsg = `Check the config file at ${humanPath(os.homedir(), req.args.config)} for the password.`
const welcomeText = req.args["welcome-text"] || (i18n.t("WELCOME", { app: appName }) as string)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this as string necessary? I would think t returns a string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the thing I'm struggling with.
Because the t returns string or null( t return type DefaultTFuncReturn).
Although I added returnNull to the i18n configuration, this config item makes the t never return null.
But the TS compiler doesn't seem to know about this, so I need to add as string.
I'd also like to know if there is a better way to avoid adding as string.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can always look in a followup PR. I don't want to hold you back so it's fine as is for now. Thank you for trying that!

let passwordMsg = i18n.t("LOGIN_PASSWORD", { configFile: humanPath(os.homedir(), req.args.config) })
if (req.args.usingEnvPassword) {
passwordMsg = "Password was set from $PASSWORD."
passwordMsg = i18n.t("LOGIN_USING_ENV_PASSWORD")
} else if (req.args.usingEnvHashedPassword) {
passwordMsg = "Password was set from $HASHED_PASSWORD."
passwordMsg = i18n.t("LOGIN_USING_HASHED_PASSWORD")
}

return replaceTemplates(
req,
content
.replace(/{{APP_NAME}}/g, appName)
.replace(/{{I18N_LOGIN_TITLE}}/g, i18n.t("LOGIN_TITLE", { app: appName }))
.replace(/{{WELCOME_TEXT}}/g, welcomeText)
.replace(/{{PASSWORD_MSG}}/g, passwordMsg)
.replace(/{{I18N_LOGIN_BELOW}}/g, i18n.t("LOGIN_BELOW"))
.replace(/{{I18N_PASSWORD_PLACEHOLDER}}/g, i18n.t("PASSWORD_PLACEHOLDER"))
.replace(/{{I18N_SUBMIT}}/g, i18n.t("SUBMIT"))
.replace(/{{ERROR}}/, error ? `<div class="error">${escapeHtml(error.message)}</div>` : ""),
)
}
Expand Down Expand Up @@ -70,11 +76,11 @@ router.post<{}, string, { password: string; base?: string }, { to?: string }>("/
try {
// Check to see if they exceeded their login attempts
if (!limiter.canTry()) {
throw new Error("Login rate limited!")
throw new Error(i18n.t("LOGIN_RATE_LIMIT") as string)
}

if (!password) {
throw new Error("Missing password")
throw new Error(i18n.t("MISS_PASSWORD") as string)
}

const passwordMethod = getPasswordMethod(hashedPasswordFromArgs)
Expand Down Expand Up @@ -108,7 +114,7 @@ router.post<{}, string, { password: string; base?: string }, { to?: string }>("/
}),
)

throw new Error("Incorrect password")
throw new Error(i18n.t("INCORRECT_PASSWORD") as string)
} catch (error: any) {
const renderedHtml = await getRoot(req, error)
res.send(renderedHtml)
Expand Down
2 changes: 2 additions & 0 deletions test/unit/node/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ describe("parser", () => {
"--verbose",
["--app-name", "custom instance name"],
["--welcome-text", "welcome to code"],
["--lng", "zh-cn"],
"2",

["--locale", "ja"],
Expand Down Expand Up @@ -131,6 +132,7 @@ describe("parser", () => {
verbose: true,
"app-name": "custom instance name",
"welcome-text": "welcome to code",
lng: "zh-cn",
version: true,
"bind-addr": "192.169.0.1:8080",
})
Expand Down
11 changes: 11 additions & 0 deletions test/unit/node/routes/login.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,5 +138,16 @@ describe("login", () => {
expect(resp.status).toBe(200)
expect(htmlContent).toContain(`Welcome to ${appName}`)
})

it("should return correct welcome text when lng is set to non-English", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for adding a test 👏🏼

process.env.PASSWORD = previousEnvPassword
const lng = "zh-cn"
const codeServer = await integration.setup([`--lng=${lng}`], "")
const resp = await codeServer.fetch("/login", { method: "GET" })

const htmlContent = await resp.text()
expect(resp.status).toBe(200)
expect(htmlContent).toContain(`欢迎来到 code-server`)
})
})
})
3 changes: 2 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
"./test/node_modules/@types",
"./lib/vscode/src/vs/server/@types"
],
"downlevelIteration": true
"downlevelIteration": true,
"resolveJsonModule": true
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is for importing the locale JSON files in typescript.

},
"include": ["./src/**/*"],
"exclude": ["/test", "/lib", "/ci", "/doc"]
Expand Down
19 changes: 19 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@
# yarn lockfile v1


"@babel/runtime@^7.20.6":
version "7.20.7"
resolved "https://registry.npmmirror.com/@babel/runtime/-/runtime-7.20.7.tgz#fcb41a5a70550e04a7b708037c7c32f7f356d8fd"
integrity sha512-UF0tvkUtxwAgZ5W/KrkHf0Rn0fdnLDU9ScxBrEVNUprE/MzirjK4MJUX1/BVDv00Sv8cljtukVK1aky++X1SjQ==
dependencies:
regenerator-runtime "^0.13.11"

"@coder/logger@^3.0.0":
version "3.0.0"
resolved "https://registry.yarnpkg.com/@coder/logger/-/logger-3.0.0.tgz#fd4d2332ca375412c75cb5ba7767d3290b106dec"
Expand Down Expand Up @@ -1814,6 +1821,13 @@ https-proxy-agent@5, https-proxy-agent@^5.0.0:
agent-base "6"
debug "4"

i18next@^22.4.6:
version "22.4.6"
resolved "https://registry.npmmirror.com/i18next/-/i18next-22.4.6.tgz#876352c3ba81bdfedc38eeda124e2bbd05f46988"
integrity sha512-9Tm1ezxWyzV+306CIDMBbYBitC1jedQyYuuLtIv7oxjp2ohh8eyxP9xytIf+2bbQfhH784IQKPSYp+Zq9+YSbw==
dependencies:
"@babel/runtime" "^7.20.6"

[email protected]:
version "0.4.24"
resolved "https://registry.yarnpkg.com/iconv-lite/-/iconv-lite-0.4.24.tgz#2022b4b25fbddc21d2f524974a474aafe733908b"
Expand Down Expand Up @@ -2873,6 +2887,11 @@ [email protected]:
resolved "https://registry.yarnpkg.com/readline-transform/-/readline-transform-1.0.0.tgz#3157f97428acaec0f05a5c1ff2c3120f4e6d904b"
integrity sha512-7KA6+N9IGat52d83dvxnApAWN+MtVb1MiVuMR/cf1O4kYsJG+g/Aav0AHcHKsb6StinayfPLne0+fMX2sOzAKg==

regenerator-runtime@^0.13.11:
version "0.13.11"
resolved "https://registry.npmmirror.com/regenerator-runtime/-/regenerator-runtime-0.13.11.tgz#f6dca3e7ceec20590d07ada785636a90cdca17f9"
integrity sha512-kY1AZVr2Ra+t+piVaJ4gxaFaReZVH40AKNo7UCX6W+dEwBo/2oZJzqfuN1qLq1oL45o56cPaTXELwrTh8Fpggg==

regexp.prototype.flags@^1.4.3:
version "1.4.3"
resolved "https://registry.yarnpkg.com/regexp.prototype.flags/-/regexp.prototype.flags-1.4.3.tgz#87cab30f80f66660181a3bb7bf5981a872b367ac"
Expand Down