Skip to content

Commit

Permalink
fix(examplebroker): Mutliple fixes to example brokers (#60)
Browse files Browse the repository at this point in the history 
Fix missing userinfo metadata when granting access. In the selection of
userinfo, we missed the marker in the data json struct in the
examplebroker. Readding it.
Fix multiple MFAs handling logic.
Removal of dead code.
Fixing cycling imports.
Labels changes.
  • Loading branch information
@didrocks
didrocks authored Sep 26, 2023
2 parents 3c19a52 + f0d6710 commit 188fe09
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 99 deletions.
123 changes: 27 additions & 96 deletions internal/brokers/examplebroker/broker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ type sessionInfo struct {

pwdChange passwdReset

neededMfa int
currentMfaStep int
neededAuthSteps int
currentAuthStep int
}

type isAuthenticatedCtx struct {
Expand All @@ -59,76 +59,13 @@ type Broker struct {
isAuthenticatedCallsMu sync.Mutex
}

type exampleUser struct {
UID string `json:"uid"`
Name string `json:"name"`
Groups map[string]map[string]string `json:"groups"`
}

func (u exampleUser) String() string {
data, err := json.Marshal(u)
if err != nil {
panic(fmt.Sprintf("Invalid user data: %v", err))
}
return string(data)
}

var (
exampleUsers = map[string]exampleUser{
"user1": {
UID: "4245874",
Name: "My user",
Groups: map[string]map[string]string{
"group1": {
"name": "Group 1",
"gid": "3884",
},
"group2": {
"name": "Group 2",
"gid": "4884",
},
},
},
"user2": {
UID: "33333",
Name: "My secondary user",
Groups: map[string]map[string]string{
"group2": {
"name": "Group 2",
"gid": "4884",
},
},
},
"user-mfa": {
UID: "44444",
Name: "User that needs MFA",
Groups: map[string]map[string]string{
"group1": {
"name": "Group 1",
"gid": "3884",
},
},
},
"user-needs-reset": {
UID: "55555",
Name: "User that needs passwd reset",
Groups: map[string]map[string]string{
"group1": {
"name": "Group 1",
"gid": "3884",
},
},
},
"user-can-reset": {
UID: "66666",
Name: "User that can passwd reset",
Groups: map[string]map[string]string{
"group1": {
"name": "Group 1",
"gid": "3884",
},
},
},
exampleUsers = map[string]string{
"user1": "My user",
"user2": "My secondary user",
"user-mfa": "User that needs MFA",
"user-needs-reset": "User that needs passwd reset",
"user-can-reset": "User that can passwd reset",
}
)

Expand All @@ -155,16 +92,18 @@ func (b *Broker) NewSession(ctx context.Context, username, lang string) (session
username: username,
lang: lang,
pwdChange: noReset,
currentAuthStep: 1,
neededAuthSteps: 1,
attemptsPerMode: make(map[string]int),
}
switch username {
case "user-mfa":
info.neededMfa = 3
info.neededAuthSteps = 3
case "user-needs-reset":
info.neededMfa = 1
info.neededAuthSteps = 2
info.pwdChange = mustReset
case "user-can-reset":
info.neededMfa = 1
info.neededAuthSteps = 2
info.pwdChange = canReset
}

Expand All @@ -185,11 +124,11 @@ func (b *Broker) GetAuthenticationModes(ctx context.Context, sessionID string, s
allModes := getSupportedModes(sessionInfo, supportedUILayouts)

// If the user needs mfa, we remove the last used mode from the list of available modes.
if sessionInfo.currentMfaStep > 0 && sessionInfo.currentMfaStep < sessionInfo.neededMfa {
if sessionInfo.currentAuthStep > 1 && sessionInfo.currentAuthStep < sessionInfo.neededAuthSteps {
allModes = getMfaModes(sessionInfo, sessionInfo.allModes)
}
// If the user needs or can reset the password, we only show those authentication modes.
if sessionInfo.currentMfaStep > 0 && sessionInfo.pwdChange != noReset {
if sessionInfo.currentAuthStep > 1 && sessionInfo.pwdChange != noReset {
allModes = getPasswdResetModes(sessionInfo, sessionInfo.allModes)
}

Expand Down Expand Up @@ -350,22 +289,22 @@ func getSupportedModes(sessionInfo sessionInfo, supportedUILayouts []map[string]
break
}
allModes["mandatoryreset"] = map[string]string{
"selection_label": "Password reset (3 days until mandatory)",
"selection_label": "Password reset",
"ui": mapToJSON(map[string]string{
"type": "newpassword",
"label": "Enter your new password",
"entry": "chars_password",
}),
}

if layout["skip-button"] != "" {
if layout["button"] != "" {
allModes["optionalreset"] = map[string]string{
"selection_label": "Password reset",
"ui": mapToJSON(map[string]string{
"type": "newpassword",
"label": "Enter your new password",
"entry": "chars_password",
"skip-button": "Skip",
"type": "newpassword",
"label": "Enter your new password (3 days until mandatory)",
"entry": "chars_password",
"button": "Skip",
}),
}
}
Expand Down Expand Up @@ -481,18 +420,10 @@ func (b *Broker) IsAuthenticated(ctx context.Context, sessionID, authenticationD
}()

access, data, err = b.handleIsAuthenticated(b.isAuthenticatedCalls[sessionID].ctx, sessionInfo, authData)
if access == responses.AuthGranted {
switch sessionInfo.username {
case "user-needs-reset":
fallthrough
case "user-can-reset":
fallthrough
case "user-mfa":
if sessionInfo.currentMfaStep < sessionInfo.neededMfa {
sessionInfo.currentMfaStep++
access = responses.AuthNext
}
}
if access == responses.AuthGranted && sessionInfo.currentAuthStep < sessionInfo.neededAuthSteps {
sessionInfo.currentAuthStep++
access = responses.AuthNext
data = ""
} else if access == responses.AuthRetry {
sessionInfo.attemptsPerMode[sessionInfo.selectedMode]++
if sessionInfo.attemptsPerMode[sessionInfo.selectedMode] >= maxAttempts {
Expand Down Expand Up @@ -603,12 +534,12 @@ func (b *Broker) handleIsAuthenticated(ctx context.Context, sessionInfo sessionI
}
}

user, exists := exampleUsers[sessionInfo.username]
name, exists := exampleUsers[sessionInfo.username]
if !exists {
return responses.AuthDenied, `{"message": "user not found"}`, nil
}

return responses.AuthGranted, userInfoFromName(user.Name), nil
return responses.AuthGranted, fmt.Sprintf(`{"userinfo": %s}`, userInfoFromName(name)), nil
}

// EndSession ends the requested session and triggers the necessary clean up steps, if any.
Expand Down
7 changes: 4 additions & 3 deletions internal/brokers/examplebroker/dbus.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ import (

"github.com/godbus/dbus/v5"
"github.com/godbus/dbus/v5/introspect"
"github.com/ubuntu/authd/internal/brokers"
"github.com/ubuntu/decorate"
)

const (
dbusObjectPath = "/com/ubuntu/authd/ExampleBroker"
busName = "com.ubuntu.authd.ExampleBroker"
// we need to redeclare the interface here to avoid include cycles.
dbusInterface = "com.ubuntu.authd.Broker"
)

// Bus is the D-Bus object that will answer calls for the broker.
Expand All @@ -34,7 +35,7 @@ func StartBus(ctx context.Context, cfgPath string) (err error) {

b, _, _ := New("ExampleBroker")
obj := Bus{broker: b}
err = conn.Export(&obj, dbusObjectPath, brokers.DbusInterface)
err = conn.Export(&obj, dbusObjectPath, dbusInterface)
if err != nil {
return err
}
Expand All @@ -44,7 +45,7 @@ func StartBus(ctx context.Context, cfgPath string) (err error) {
Interfaces: []introspect.Interface{
introspect.IntrospectData,
{
Name: brokers.DbusInterface,
Name: dbusInterface,
Methods: introspect.Methods(&obj),
},
},
Expand Down

0 comments on commit 188fe09

Please sign in to comment.