Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
9 changes: 8 additions & 1 deletion cmd/cloud/regions/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,14 @@ func (c *ListController) Run(cmd *cobra.Command, args []string) (fctl.Renderable
return nil, err
}

organizationID, apiClient, err := fctl.NewMembershipClientForOrganizationFromFlags(cmd, relyingParty, fctl.NewPTermDialog(), profileName, *profile)
organizationID, apiClient, err := fctl.NewMembershipClientForOrganizationFromFlagsWithScopes(
cmd,
relyingParty,
fctl.NewPTermDialog(),
profileName,
*profile,
[]string{"organization:ListRegions"},
)
if err != nil {
return nil, err
}
Expand Down
30 changes: 26 additions & 4 deletions cmd/stack/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,19 @@ func (c *CreateController) Run(cmd *cobra.Command, args []string) (fctl.Renderab
return nil, err
}

organizationID, apiClient, err := fctl.NewMembershipClientForOrganizationFromFlags(cmd, relyingParty, fctl.NewPTermDialog(), profileName, *profile)
region := fctl.GetString(cmd, regionFlag)
requiredScopes := []string{
"organization:CreateStack",
"organization:ReadRegion",
}
if region == "" {
requiredScopes = append(requiredScopes, "organization:ListRegions")
}
if !fctl.GetBool(cmd, nowaitFlag) {
requiredScopes = append(requiredScopes, "organization:ReadStack")
}

organizationID, apiClient, err := fctl.NewMembershipClientForOrganizationFromFlagsWithScopes(cmd, relyingParty, fctl.NewPTermDialog(), profileName, *profile, requiredScopes)
if err != nil {
return nil, err
}
Expand All @@ -85,7 +97,6 @@ func (c *CreateController) Run(cmd *cobra.Command, args []string) (fctl.Renderab
}
}

region := fctl.GetString(cmd, regionFlag)
if region == "" {
listRegionsRequest := operations.ListRegionsRequest{
OrganizationID: organizationID,
Expand Down Expand Up @@ -152,7 +163,7 @@ func (c *CreateController) Run(cmd *cobra.Command, args []string) (fctl.Renderab
specifiedVersion := fctl.GetString(cmd, versionFlag)
if specifiedVersion == "" {
var options []string
for _, version := range availableVersionsResponse.GetRegionVersionsResponse.GetData() {
for _, version := range sortRegionVersionsByLatest(availableVersionsResponse.GetRegionVersionsResponse.GetData()) {
options = append(options, version.GetName())
}

Expand Down Expand Up @@ -219,7 +230,18 @@ func (c *CreateController) Run(cmd *cobra.Command, args []string) (fctl.Renderab

fctl.BasicTextCyan.WithWriter(cmd.OutOrStdout()).Println("Your portal will be reachable on: " + portal)

// todo: need a long running client with auto refresh
if !profile.RootTokens.ID.Claims.HasStackAccess(organizationID, stackData.GetID()) {
fctl.NewPTermDialog().Info("Refreshing profile accesses...")
tokens, err := fctl.RefreshRootTokens(cmd.Context(), relyingParty, *profile.RootTokens)
if err != nil {
return nil, fmt.Errorf("refreshing profile accesses: %w", err)
}
profile.UpdateRootToken(tokens)
if err := fctl.WriteProfile(cmd, profileName, *profile); err != nil {
return nil, fmt.Errorf("writing refreshed profile: %w", err)
}
}

stackClient, err := fctl.NewStackClient(
cmd,
relyingParty,
Expand Down
5 changes: 2 additions & 3 deletions cmd/stack/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

"github.com/pterm/pterm"
"github.com/spf13/cobra"
"golang.org/x/mod/semver"

"github.com/formancehq/fctl/internal/membershipclient/v3"
"github.com/formancehq/fctl/internal/membershipclient/v3/models/components"
Expand Down Expand Up @@ -181,9 +180,9 @@ func retrieveUpgradableVersion(ctx context.Context, organization string, stack c
if versionName == *currentVersion {
continue
}
if !semver.IsValid(versionName) || semver.Compare(versionName, *currentVersion) >= 1 {
if isVersionNewerThanCurrent(versionName, *currentVersion) {
upgradeOptions = append(upgradeOptions, versionName)
}
}
return upgradeOptions, nil
return sortVersionNamesByLatest(upgradeOptions), nil
}
91 changes: 91 additions & 0 deletions cmd/stack/version_sort.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package stack

import (
"sort"
"strings"

"golang.org/x/mod/semver"

"github.com/formancehq/fctl/internal/membershipclient/v3/models/components"
)

func sortRegionVersionsByLatest(versions []components.Version) []components.Version {
sorted := append([]components.Version(nil), versions...)
sort.SliceStable(sorted, func(i, j int) bool {
return compareVersionNamesByLatest(sorted[i].GetName(), sorted[j].GetName())
})
return sorted
}

func sortVersionNamesByLatest(versions []string) []string {
sorted := append([]string(nil), versions...)
sort.SliceStable(sorted, func(i, j int) bool {
return compareVersionNamesByLatest(sorted[i], sorted[j])
})
return sorted
}

func compareVersionNamesByLatest(a, b string) bool {
normalizedA, validA := normalizeSemver(a)
normalizedB, validB := normalizeSemver(b)

if validA && validB {
return semver.Compare(normalizedA, normalizedB) > 0
}
if validA != validB {
return validA
}
return a > b
}

func isVersionNewerThanCurrent(candidate, current string) bool {
normalizedCandidate, validCandidate := normalizeSemver(candidate)
normalizedCurrent, validCurrent := normalizeSemver(current)
if validCandidate && validCurrent {
return semver.Compare(normalizedCandidate, normalizedCurrent) > 0
}
return true
}
Comment on lines +41 to +48

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Invalid candidate versions are always treated as newer.

When candidate is not semver-parseable but current is valid, the function returns true, incorrectly presenting invalid versions as upgrade candidates. This could lead to users selecting non-semver versions (e.g., typos, placeholder strings) as upgrade targets.

Consider returning false when the candidate is invalid:

🐛 Proposed fix
 func isVersionNewerThanCurrent(candidate, current string) bool {
 	normalizedCandidate, validCandidate := normalizeSemver(candidate)
 	normalizedCurrent, validCurrent := normalizeSemver(current)
 	if validCandidate && validCurrent {
 		return semver.Compare(normalizedCandidate, normalizedCurrent) > 0
 	}
-	return true
+	// If candidate is not parseable, don't treat it as newer
+	// If current is not parseable, any valid candidate is acceptable
+	return validCandidate
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func isVersionNewerThanCurrent(candidate, current string) bool {
normalizedCandidate, validCandidate := normalizeSemver(candidate)
normalizedCurrent, validCurrent := normalizeSemver(current)
if validCandidate && validCurrent {
return semver.Compare(normalizedCandidate, normalizedCurrent) > 0
}
return true
}
func isVersionNewerThanCurrent(candidate, current string) bool {
normalizedCandidate, validCandidate := normalizeSemver(candidate)
normalizedCurrent, validCurrent := normalizeSemver(current)
if validCandidate && validCurrent {
return semver.Compare(normalizedCandidate, normalizedCurrent) > 0
}
// If candidate is not parseable, don't treat it as newer
// If current is not parseable, any valid candidate is acceptable
return validCandidate
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/stack/version_sort.go` around lines 41 - 48, The function
isVersionNewerThanCurrent currently treats any non-parseable candidate as newer;
change it so that if normalizeSemver reports the candidate is invalid you return
false (do not treat invalid candidates as upgrades). Keep the existing behavior
of comparing normalizedCandidate and normalizedCurrent when both valid, and if
candidate is valid but current is not you may still return true; update the
logic in isVersionNewerThanCurrent (which calls normalizeSemver and
semver.Compare) so invalid candidate -> false, validCandidate && validCurrent ->
use semver.Compare, otherwise (validCandidate && !validCurrent) -> true.


func normalizeSemver(version string) (string, bool) {
version = strings.TrimSpace(version)
if version == "" {
return "", false
}
if !strings.HasPrefix(version, "v") {
version = "v" + version
}
if semver.IsValid(version) {
return version, true
}

suffixStart := len(version)
for _, separator := range []string{"-", "+"} {
if index := strings.Index(version, separator); index >= 0 && index < suffixStart {
suffixStart = index
}
}

core := strings.TrimPrefix(version[:suffixStart], "v")
suffix := version[suffixStart:]
parts := strings.Split(core, ".")
if len(parts) > 3 {
return "", false
}
for len(parts) < 3 {
parts = append(parts, "0")
}
for _, part := range parts {
if part == "" {
return "", false
}
for _, r := range part {
if r < '0' || r > '9' {
return "", false
}
}
}

normalized := "v" + strings.Join(parts, ".") + suffix
return normalized, semver.IsValid(normalized)
}
56 changes: 56 additions & 0 deletions cmd/stack/version_sort_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package stack

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/formancehq/fctl/internal/membershipclient/v3/models/components"
)

func TestSortRegionVersionsByLatest(t *testing.T) {
versions := []components.Version{
{Name: "v1.2.0"},
{Name: "1.10.0"},
{Name: "v2.0.0-rc.1"},
{Name: "v2.0.0"},
{Name: "v1.9.0"},
}

sorted := sortRegionVersionsByLatest(versions)

require.Equal(t, []string{
"v2.0.0",
"v2.0.0-rc.1",
"1.10.0",
"v1.9.0",
"v1.2.0",
}, []string{
sorted[0].GetName(),
sorted[1].GetName(),
sorted[2].GetName(),
sorted[3].GetName(),
sorted[4].GetName(),
})
require.Equal(t, "v1.2.0", versions[0].GetName())
}

func TestSortVersionNamesByLatest(t *testing.T) {
require.Equal(t,
[]string{"v1.11.0", "1.10.0", "v1.2.0"},
sortVersionNamesByLatest([]string{"v1.2.0", "v1.11.0", "1.10.0"}),
)
}

func TestSortVersionNamesByLatestWithShortVersions(t *testing.T) {
require.Equal(t,
[]string{"v3.2-rc", "v3.1", "v3.0", "v2.2", "v2.1"},
sortVersionNamesByLatest([]string{"v3.0", "v2.2", "v3.1", "v3.2-rc", "v2.1"}),
)
}

func TestIsVersionNewerThanCurrent(t *testing.T) {
require.True(t, isVersionNewerThanCurrent("1.10.0", "v1.9.0"))
require.False(t, isVersionNewerThanCurrent("v1.9.0", "1.10.0"))
require.True(t, isVersionNewerThanCurrent("v3.2-rc", "v3.1"))
}
58 changes: 58 additions & 0 deletions pkg/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,44 @@ func Refresh(ctx context.Context, relyingParty client.RelyingParty, token Access
}, nil
}

func RefreshRootTokens(ctx context.Context, relyingParty client.RelyingParty, tokens Tokens) (*Tokens, error) {
newToken, err := client.RefreshTokens[*IDTokenClaims](ctx, relyingParty, tokens.Access.Refresh, "", "")
if err != nil {
return nil, newErrInvalidAuthentication(err)
}

accessTokenClaims := AccessTokenClaims{}
if _, err := oidc.ParseToken(newToken.AccessToken, &accessTokenClaims); err != nil {
return nil, newErrInvalidAuthentication(err)
}

refreshToken := newToken.RefreshToken
if refreshToken == "" {
refreshToken = tokens.Access.Refresh
}

idToken := tokens.ID.Token
idTokenClaims := tokens.ID.Claims
if newToken.IDToken != "" && newToken.IDTokenClaims != nil {
idToken = newToken.IDToken
idTokenClaims = *newToken.IDTokenClaims
}

return &Tokens{
Access: AccessToken{
TokenWithClaims: TokenWithClaims[AccessTokenClaims]{
Token: newToken.AccessToken,
Claims: accessTokenClaims,
},
Refresh: refreshToken,
},
ID: IDToken{
Token: idToken,
Claims: idTokenClaims,
},
}, nil
}

func FetchStackToken(ctx context.Context, httpClient *http.Client, stackURI, token string) (*oauth2.Token, error) {
form := url.Values{
"grant_type": []string{"urn:ietf:params:oauth:grant-type:jwt-bearer"},
Expand Down Expand Up @@ -323,6 +361,26 @@ type AccessToken struct {
Refresh string `json:"refreshToken"`
}

func (t AccessToken) MissingScopes(scopes ...string) []string {
tokenScopes := make(map[string]struct{}, len(t.Claims.Scopes))
for _, scope := range t.Claims.Scopes {
tokenScopes[scope] = struct{}{}
}

missingScopes := make([]string, 0)
for _, scope := range scopes {
if _, ok := tokenScopes[scope]; !ok {
missingScopes = append(missingScopes, scope)
}
}

return missingScopes
}

func (t AccessToken) HasScopes(scopes ...string) bool {
return len(t.MissingScopes(scopes...)) == 0
}

func (t AccessToken) ToOAuth2() *oauth2.Token {
return &oauth2.Token{
AccessToken: t.Token,
Expand Down
40 changes: 40 additions & 0 deletions pkg/authentication_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package fctl

import (
"reflect"
"testing"

"github.com/formancehq/go-libs/v4/oidc"
)

func TestAccessTokenMissingScopes(t *testing.T) {
token := AccessToken{
TokenWithClaims: TokenWithClaims[AccessTokenClaims]{
Claims: AccessTokenClaims{
Scopes: oidc.SpaceDelimitedArray{
"organization:CreateStack",
"organization:ReadRegion",
},
},
},
}

missingScopes := token.MissingScopes(
"organization:CreateStack",
"organization:ListRegions",
"organization:ReadRegion",
)

expected := []string{"organization:ListRegions"}
if !reflect.DeepEqual(missingScopes, expected) {
t.Fatalf("expected missing scopes %v, got %v", expected, missingScopes)
}

if token.HasScopes("organization:CreateStack", "organization:ListRegions") {
t.Fatal("expected HasScopes to return false for missing scope")
}

if !token.HasScopes("organization:CreateStack", "organization:ReadRegion") {
t.Fatal("expected HasScopes to return true when all scopes are present")
}
}
Loading
Loading