Skip to content

Commit 75cdd4c

Browse files
celdrakeceldrake
committed
Fixes from code review
1 parent 773ccb1 commit 75cdd4c

File tree

6 files changed

+115
-42
lines changed

6 files changed

+115
-42
lines changed

‎libs/i18n/locales/en/translation.json‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -592,10 +592,7 @@
592592
"With the {{ brandName }} command line interface, you can manage your fleets, devices and repositories from a terminal.": "With the {{ brandName }} command line interface, you can manage your fleets, devices and repositories from a terminal.",
593593
"Command line tools are not available for download in this {{ brandName }} installation.": "Command line tools are not available for download in this {{ brandName }} installation.",
594594
"Login successful!": "Login successful!",
595-
"Copy and run this command in your terminal to authenticate to {{ brandName }}:": "Copy and run this command in your terminal to authenticate to {{ brandName }}:",
596595
"Show more": "Show more",
597-
"Show Less": "Show Less",
598-
"Show More": "Show More",
599596
"Failed to obtain session token": "Failed to obtain session token",
600597
"This URL can only be used with a valid session ID": "This URL can only be used with a valid session ID",
601598
"The login command for this session is no longer available": "The login command for this session is no longer available",

‎libs/ui-components/src/components/Masthead/CopyLoginCommandPage.tsx‎

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,16 @@ const LoginCommandCopy = ({ loginCommand, brandName }: { loginCommand: string; b
3838
const { t } = useTranslation();
3939
const [isExpanded, setIsExpanded] = React.useState(false);
4040

41-
// Truncate the command for initial display (show first part + ellipsis)
42-
const truncatedCommand =
43-
loginCommand.length > TRUNCATED_COMMAND_LENGTH
44-
? `${loginCommand.substring(0, TRUNCATED_COMMAND_LENGTH)}`
45-
: loginCommand;
41+
const hasShowMore = loginCommand.length > TRUNCATED_COMMAND_LENGTH;
42+
const visibleCommand = hasShowMore ? `${loginCommand.substring(0, TRUNCATED_COMMAND_LENGTH)}...` : loginCommand;
4643

4744
return (
4845
<Stack hasGutter className="fctl-login-command__copy-content">
4946
<StackItem>
5047
<Alert variant="success" isInline title={t('Login successful!')} />
5148
</StackItem>
5249
<StackItem>
53-
{t('Copy and run this command in your terminal to authenticate to {{ brandName }}:', { brandName })}
50+
{t('Copy and run this command in your terminal to authenticate with {{ brandName }}:', { brandName })}
5451
</StackItem>
5552
<StackItem>
5653
<CodeBlock
@@ -62,23 +59,27 @@ const LoginCommandCopy = ({ loginCommand, brandName }: { loginCommand: string; b
6259
>
6360
<CodeBlockCode className="fctl-login-command__codeblock">
6461
{/* When the full command is shown, hide the truncated command. This allows the full command to be copied using the keyboard */}
65-
{isExpanded ? '' : `${truncatedCommand}...`}
66-
<ExpandableSection
67-
toggleText={isExpanded ? t('Show less') : t('Show more')}
68-
isExpanded={isExpanded}
69-
isDetached
70-
contentId="code-block-expand"
71-
>
72-
{loginCommand}
73-
</ExpandableSection>
74-
<ExpandableSectionToggle
75-
isExpanded={isExpanded}
76-
onToggle={(isExpanded) => setIsExpanded(isExpanded)}
77-
contentId="code-block-expand"
78-
direction={isExpanded ? 'up' : 'down'}
79-
>
80-
{isExpanded ? t('Show Less') : t('Show More')}
81-
</ExpandableSectionToggle>
62+
{isExpanded ? '' : visibleCommand}
63+
{hasShowMore && (
64+
<>
65+
<ExpandableSection
66+
toggleText={isExpanded ? t('Show less') : t('Show more')}
67+
isExpanded={isExpanded}
68+
isDetached
69+
contentId="code-block-expand"
70+
>
71+
{loginCommand}
72+
</ExpandableSection>
73+
<ExpandableSectionToggle
74+
isExpanded={isExpanded}
75+
onToggle={(isExpanded) => setIsExpanded(isExpanded)}
76+
contentId="code-block-expand"
77+
direction={isExpanded ? 'up' : 'down'}
78+
>
79+
{isExpanded ? t('Show less') : t('Show more')}
80+
</ExpandableSectionToggle>
81+
</>
82+
)}
8283
</CodeBlockCode>
8384
</CodeBlock>
8485
</StackItem>
@@ -101,7 +102,7 @@ const CopyLoginCommandBody = ({ brandName }: { brandName: string }) => {
101102
const [loginCommand, setLoginCommand] = React.useState('');
102103
const [tokenNotFound, setTokenNotFound] = React.useState(false);
103104
const [errorMessage, setErrorMessage] = React.useState('');
104-
const sessionId = window.location.search.split('sessionId=')[1];
105+
const sessionId = new URLSearchParams(window.location.search).get('sessionId');
105106

106107
React.useEffect(() => {
107108
const getSessionToken = async () => {

‎libs/ui-components/src/components/common/CopyButton.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,16 @@ const CopyButton = ({ ariaLabel, text, variant }: CopyButtonProps) => {
2121
};
2222

2323
React.useEffect(() => {
24-
let timeout: NodeJS.Timeout;
24+
let timeout: ReturnType<typeof setTimeout> | undefined;
2525
if (copied) {
2626
timeout = setTimeout(() => {
2727
setCopied(false);
2828
}, 1000);
2929
}
3030
return () => {
31-
clearTimeout(timeout);
31+
if (timeout) {
32+
clearTimeout(timeout);
33+
}
3234
};
3335
}, [copied]);
3436

@@ -37,9 +39,10 @@ const CopyButton = ({ ariaLabel, text, variant }: CopyButtonProps) => {
3739
<Button
3840
variant={variant || 'plain'}
3941
isInline={variant === 'link'}
42+
onClick={onCopy}
4043
icon={
4144
<Icon size="sm">
42-
<CopyIcon onClick={onCopy} />
45+
<CopyIcon />
4346
</Icon>
4447
}
4548
aria-label={ariaLabel || t('Copy text')}

‎libs/ui-components/src/components/common/CopyLoginCommandModal.tsx‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const CopyLoginCommandModal = ({ onClose }: { onClose: VoidFunction }) => {
2323
const brandName = getBrandName(settings);
2424

2525
const [serviceUrl, setServiceUrl] = React.useState('');
26-
const loginCommand = String.raw`flightctl login ${serviceUrl || DEFAULT_API_URL} --token=$(oc whoami -t)`;
26+
const loginCommand = serviceUrl ? String.raw`flightctl login ${serviceUrl} --token=$(oc whoami -t)` : '';
2727

2828
React.useEffect(() => {
2929
const loadServiceUrl = async () => {
@@ -50,13 +50,17 @@ const CopyLoginCommandModal = ({ onClose }: { onClose: VoidFunction }) => {
5050
</StackItem>
5151
<StackItem>
5252
<CodeBlock
53-
actions={[
54-
<CodeBlockAction key="copy-command">
55-
<CopyButton text={loginCommand} ariaLabel={t('Copy login command')} />
56-
</CodeBlockAction>,
57-
]}
53+
actions={
54+
loginCommand
55+
? [
56+
<CodeBlockAction key="copy-command">
57+
<CopyButton text={loginCommand} ariaLabel={t('Copy login command')} />
58+
</CodeBlockAction>,
59+
]
60+
: undefined
61+
}
5862
>
59-
<CodeBlockCode>{serviceUrl ? loginCommand : t('Loading...')}</CodeBlockCode>
63+
<CodeBlockCode>{loginCommand || t('Loading...')}</CodeBlockCode>
6064
</CodeBlock>
6165
</StackItem>
6266
<StackItem>

‎libs/ui-components/src/types/extraTypes.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,5 +80,5 @@ export enum AuthType {
8080
DISABLED = 'DISABLED',
8181
OIDC = 'OIDC',
8282
K8S = 'K8S',
83-
AAP = 'AAP',
83+
AAP = 'AAPGateway',
8484
}

‎proxy/auth/auth.go‎

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ import (
1616
"github.com/flightctl/flightctl/api/v1alpha1"
1717
)
1818

19+
const (
20+
DefaultTokenCleanupInterval = 10 * time.Minute // How often to run cleanup
21+
DefaultTokenMaxAge = 15 * time.Minute // Max age before forced removal
22+
)
23+
1924
type ExpiresInResp struct {
2025
ExpiresIn *int64 `json:"expiresIn"`
2126
}
@@ -35,9 +40,10 @@ type RedirectResponse struct {
3540
}
3641

3742
type AuthHandler struct {
38-
provider AuthProvider
39-
sessionMutex sync.Mutex
40-
apiTokenMap map[string]*ApiToken
43+
provider AuthProvider
44+
sessionMutex sync.Mutex
45+
apiTokenMap map[string]*ApiToken
46+
stopTokenCleanup chan struct{}
4147
}
4248

4349
type ApiToken struct {
@@ -48,7 +54,8 @@ type ApiToken struct {
4854

4955
func NewAuth(apiTlsConfig *tls.Config) (*AuthHandler, error) {
5056
auth := AuthHandler{
51-
apiTokenMap: make(map[string]*ApiToken),
57+
apiTokenMap: make(map[string]*ApiToken),
58+
stopTokenCleanup: make(chan struct{}),
5259
}
5360
authConfig, internalAuthUrl, err := getAuthInfo(apiTlsConfig)
5461
if err != nil {
@@ -68,6 +75,12 @@ func NewAuth(apiTlsConfig *tls.Config) (*AuthHandler, error) {
6875
default:
6976
err = fmt.Errorf("unknown auth type: %s", authConfig.AuthType)
7077
}
78+
79+
if err == nil && auth.provider != nil {
80+
// Start the cleanup routine for expired tokens
81+
go auth.startTokenCleanup()
82+
}
83+
7184
return &auth, err
7285
}
7386

@@ -223,6 +236,61 @@ func (a *AuthHandler) getSingleUseApiTokenMapping(sessionId string) (*ApiToken,
223236
return session, exists
224237
}
225238

239+
// Cleans up expired and old tokens that were never retrieved through getSingleUseApiTokenMapping
240+
func (a *AuthHandler) startTokenCleanup() {
241+
ticker := time.NewTicker(DefaultTokenCleanupInterval)
242+
defer ticker.Stop()
243+
244+
for {
245+
select {
246+
case <-ticker.C:
247+
a.cleanupExpiredTokens()
248+
case <-a.stopTokenCleanup:
249+
return
250+
}
251+
}
252+
}
253+
254+
// Removes expired tokens and tokens older than DefaultMaxTokenAge from the apiTokenMap
255+
func (a *AuthHandler) cleanupExpiredTokens() {
256+
a.sessionMutex.Lock()
257+
defer a.sessionMutex.Unlock()
258+
259+
now := time.Now()
260+
deletedTokens := 0
261+
262+
for sessionId, token := range a.apiTokenMap {
263+
shouldRemove := false
264+
265+
if now.Sub(token.CreatedAt) > DefaultTokenMaxAge {
266+
// Remove tokens older than the directed max age
267+
shouldRemove = true
268+
} else if token.ExpiresIn != nil {
269+
// Remove expired tokens (if ExpiresIn is set)
270+
expiration := token.CreatedAt.Add(time.Duration(*token.ExpiresIn) * time.Second)
271+
if now.After(expiration) {
272+
shouldRemove = true
273+
}
274+
}
275+
276+
if shouldRemove {
277+
delete(a.apiTokenMap, sessionId)
278+
deletedTokens++
279+
}
280+
}
281+
282+
if deletedTokens > 0 {
283+
log.GetLogger().WithFields(map[string]interface{}{
284+
"deletedTokens": deletedTokens,
285+
"remainingTokens": len(a.apiTokenMap),
286+
}).Info("Cleaned up expired API tokens")
287+
}
288+
}
289+
290+
func (a *AuthHandler) StopTokenCleanup() {
291+
close(a.stopTokenCleanup)
292+
}
293+
226294
func (a *AuthHandler) CreateSessionToken(w http.ResponseWriter, r *http.Request) {
227295
if a.provider == nil {
228296
w.WriteHeader(http.StatusTeapot)

0 commit comments

Comments
 (0)