Skip to content

Commit

Permalink
Fix bug in hostlist API: Use ACL when grouping
Browse files Browse the repository at this point in the history
Fixes #62.
Also, a few other minor fixes (see diff)
  • Loading branch information
oyvindhagberg committed Dec 12, 2018
1 parent 5eb099d commit 5d90e64
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 7 deletions.
17 changes: 17 additions & 0 deletions server/service/accessControl.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"io/ioutil"
"net/http"
"net/url"
"strings"
)

// AccessProfile holds information about which hosts the user is allowed access to,
Expand All @@ -22,6 +23,22 @@ func (ap *AccessProfile) IsAdmin() bool {
return ap.isAdmin
}

func (ap *AccessProfile) GetSQLWHERE() string {
//TODO this is temporary (bad) solution; will be removed later. See issue #67
var sql strings.Builder
sql.WriteString("'")
frist := true
for k := range ap.certs {
if !frist {
sql.WriteString("','")
}
sql.WriteString(k)
frist = false
}
sql.WriteString("'")
return sql.String()
}

func GenerateAccessProfileForUser(userID string) (*AccessProfile, error) {
if authorizationPluginURL == "" {
// If no authorization plugin is defined,
Expand Down
9 changes: 7 additions & 2 deletions server/service/api_hostlist.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (vars *apiMethodHostList) ServeHTTP(w http.ResponseWriter, req *http.Reques
// Grouping changes the whole SQL statement and what's returned,
// so it is handled in a separate function
if req.FormValue("group") != "" {
performGroupQuery(w, req, vars.db, customFieldIDs, vars.devmode)
performGroupQuery(w, req, vars.db, customFieldIDs, vars.devmode, access)
return
}

Expand Down Expand Up @@ -305,7 +305,7 @@ func (vars *apiMethodHostList) ServeHTTP(w http.ResponseWriter, req *http.Reques
}

func performGroupQuery(w http.ResponseWriter, req *http.Request,
db *sql.DB, customFieldIDs map[string]int, devmode bool) {
db *sql.DB, customFieldIDs map[string]int, devmode bool, access *AccessProfile) {
if req.FormValue("fields") != "" {
http.Error(w, "Can't combine group and fields parameters",
http.StatusUnprocessableEntity)
Expand Down Expand Up @@ -362,6 +362,11 @@ func performGroupQuery(w http.ResponseWriter, req *http.Request,

if len(where) > 0 {
statement += " WHERE " + where
if access != nil && !access.IsAdmin() {
statement += " AND certfp IN (" + access.GetSQLWHERE() + ")"
}
} else if access != nil && !access.IsAdmin() {
statement += " WHERE certfp IN (" + access.GetSQLWHERE() + ")"
}
statement += " GROUP BY " + colname

Expand Down
14 changes: 14 additions & 0 deletions server/service/api_hostlist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,20 @@ func TestApiMethodHostList(t *testing.T) {
expectStatus: http.StatusOK,
expectJSON: "{\"workstation\":2}",
},
// Test with an access profile that should prevent some hosts from being counted
{
methodAndPath: "GET /api/v0/hostlist?group=osEdition",
expectStatus: http.StatusOK,
expectJSON: "{\"workstation\":1}",
accessProfile: &AccessProfile{isAdmin: false, certs: map[string]bool{"1111": true}},
},
// Test with an access profile that should prevent some hosts from being counted
{
methodAndPath: "GET /api/v0/hostlist?group=osEdition&hostname=*baz*",
expectStatus: http.StatusOK,
expectJSON: "{}",
accessProfile: &AccessProfile{isAdmin: false, certs: map[string]bool{"1111": true}},
},
}

db := getDBconnForTesting(t)
Expand Down
8 changes: 6 additions & 2 deletions server/service/api_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,14 @@ func countFiles(w http.ResponseWriter, req *http.Request) {
return
}
i, err := strconv.Atoi(req.FormValue("n"))
if err != nil || i == 0 {
if err != nil {
http.Error(w, "Invalid number: "+req.FormValue("n"), http.StatusBadRequest)
return
}
pfib.Add(float64(i)) // pfib = parsed files interval buffer
if i > 0 {
pfib.Add(float64(i)) // pfib = parsed files interval buffer
}
http.Error(w, "OK", http.StatusNoContent)
}

func doNothing(w http.ResponseWriter, req *http.Request) {
Expand Down
4 changes: 2 additions & 2 deletions server/service/fastSearch.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ func compareSearchCacheToDB(db *sql.DB) {
log.Panic(rows.Err())
}
// find entries in the cache that should have been removed
fsMutex.RLock()
defer fsMutex.RUnlock()
fsMutex.Lock()
defer fsMutex.Unlock()
var obsoleteCount int
for fileID := range fsKey {
if _, ok := source[fileID]; !ok {
Expand Down
4 changes: 3 additions & 1 deletion server/service/oauth2login.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ func handleOauth2Redirect(w http.ResponseWriter, req *http.Request) {
http.Error(w, "Error reading Userinfo from Oauth2 provider", http.StatusInternalServerError)
return
}
log.Printf("Oauth2: Userinfo: %s", string(body))
if devmode {
log.Printf("Oauth2: Userinfo: %s", string(body))
}

// Parse the JSON
var userinfo interface{}
Expand Down

0 comments on commit 5d90e64

Please sign in to comment.