Skip to content

Commit 04b829e

Browse files
author
Julian Koberg
committed
feat: expand mfa checks also to queries
Signed-off-by: Julian Koberg <[email protected]>
1 parent dda6104 commit 04b829e

File tree

6 files changed

+90
-63
lines changed

6 files changed

+90
-63
lines changed

ocis-pkg/mfa/mfa.go

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,29 +22,20 @@ var mfaKey = mfaKeyType{}
2222
// This operation does not overwrite existing context values.
2323
func EnhanceRequest(req *http.Request) *http.Request {
2424
ctx := req.Context()
25-
if Get(ctx) {
25+
if Has(ctx) {
2626
return req
2727
}
2828
return req.WithContext(Set(ctx, req.Header.Get(MFAHeader) == "true"))
2929
}
3030

31-
// EnsureOrReject sets the MFA required header and status code 403 if not multi factor authenticated.
32-
func EnsureOrReject(ctx context.Context, w http.ResponseWriter) (hasMFA bool) {
33-
hasMFA = Get(ctx)
34-
if !hasMFA {
35-
SetRequiredStatus(w)
36-
}
37-
return
38-
}
39-
4031
// SetRequiredStatus sets the MFA required header and the statuscode to 403
4132
func SetRequiredStatus(w http.ResponseWriter) {
4233
w.Header().Set(MFARequiredHeader, "true")
4334
w.WriteHeader(http.StatusForbidden)
4435
}
4536

46-
// Get gets the mfa status from the context.
47-
func Get(ctx context.Context) bool {
37+
// Has returns the mfa status from the context.
38+
func Has(ctx context.Context) bool {
4839
mfa, ok := ctx.Value(mfaKey).(bool)
4940
if !ok {
5041
return false

ocis-pkg/mfa/mfa_test.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,10 @@ func exampleUsage() http.HandlerFunc {
1919
ctx := r.Context()
2020

2121
// now you can check if the user has MFA enabled
22-
hasMFA := mfa.Get(ctx)
23-
_ = hasMFA // use it as needed
24-
25-
// use convenience method to automatically set headers and response codes
26-
if !mfa.EnsureOrReject(ctx, w) {
22+
if !mfa.Has(ctx) {
2723
// use this line to log access denied information
2824
// mfa package will not log anything by itself
25+
mfa.SetRequiredStatus(w)
2926
return
3027
}
3128
// user has MFA enabled, you can now proceed with sensitive operation

services/graph/pkg/service/v0/drives.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,10 @@ func (g Graph) GetAllDrives(version APIVersion) http.HandlerFunc {
133133
// GetAllDrivesV1 attempts to retrieve the current users drives;
134134
// it includes another user's drives, if the current user has the permission.
135135
func (g Graph) GetAllDrivesV1(w http.ResponseWriter, r *http.Request) {
136-
if !mfa.EnsureOrReject(r.Context(), w) {
136+
if !mfa.Has(r.Context()) {
137137
logger := g.logger.SubloggerWithRequestID(r.Context())
138138
logger.Error().Str("path", r.URL.Path).Msg("MFA required but not satisfied")
139+
mfa.SetRequiredStatus(w)
139140
return
140141
}
141142

@@ -159,9 +160,10 @@ func (g Graph) GetAllDrivesV1(w http.ResponseWriter, r *http.Request) {
159160
// it includes the grantedtoV2 property
160161
// it uses unified roles instead of the cs3 representations
161162
func (g Graph) GetAllDrivesV1Beta1(w http.ResponseWriter, r *http.Request) {
162-
if !mfa.EnsureOrReject(r.Context(), w) {
163+
if !mfa.Has(r.Context()) {
163164
logger := g.logger.SubloggerWithRequestID(r.Context())
164165
logger.Error().Str("path", r.URL.Path).Msg("MFA required but not satisfied")
166+
mfa.SetRequiredStatus(w)
165167
return
166168
}
167169

services/graph/pkg/service/v0/graph.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"path"
99
"strings"
1010

11+
"github.com/CiscoM31/godata"
1112
gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
1213
storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
1314
"github.com/go-chi/chi/v5"
@@ -152,3 +153,39 @@ func parseIDParam(r *http.Request, param string) (storageprovider.ResourceId, er
152153
}
153154
return id, nil
154155
}
156+
157+
// regular users can only search for terms with a minimum length
158+
func hasAcceptableSearch(query *godata.GoDataQuery, minSearchLength int) bool {
159+
if query == nil || query.Search == nil {
160+
return false
161+
}
162+
163+
if strings.HasPrefix(query.Search.RawValue, "\"") {
164+
// if search starts with double quotes then it must finish with double quotes
165+
// add +2 to the minimum search length in this case
166+
minSearchLength += 2
167+
}
168+
169+
return len(query.Search.RawValue) >= minSearchLength
170+
}
171+
172+
// regular users can only filter by userType
173+
func hasAcceptableFilter(query *godata.GoDataQuery) bool {
174+
switch {
175+
case query == nil || query.Filter == nil:
176+
return true
177+
case query.Filter.Tree.Token.Type != godata.ExpressionTokenLogical:
178+
return false
179+
case query.Filter.Tree.Token.Value != "eq":
180+
return false
181+
case query.Filter.Tree.Children[0].Token.Value != "userType":
182+
return false
183+
}
184+
185+
return true
186+
}
187+
188+
// regular users can only use basic queries without any expansions, computes or applies
189+
func hasAcceptableQuery(query *godata.GoDataQuery) bool {
190+
return query != nil && query.Apply == nil && query.Expand == nil && query.Compute == nil
191+
}

services/graph/pkg/service/v0/groups.go

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,35 +33,34 @@ func (g Graph) GetGroups(w http.ResponseWriter, r *http.Request) {
3333
return
3434
}
3535
ctxHasFullPerms := g.contextUserHasFullAccountPerms(r.Context())
36-
searchHasAcceptableLength := false
37-
if odataReq.Query != nil && odataReq.Query.Search != nil {
38-
minSearchLength := g.config.API.IdentitySearchMinLength
39-
if strings.HasPrefix(odataReq.Query.Search.RawValue, "\"") {
40-
// if search starts with double quotes then it must finish with double quotes
41-
// add +2 to the minimum search length in this case
42-
minSearchLength += 2
43-
}
44-
searchHasAcceptableLength = len(odataReq.Query.Search.RawValue) >= minSearchLength
45-
}
46-
if !searchHasAcceptableLength {
36+
hasMFA := mfa.Has(r.Context())
37+
if !hasAcceptableSearch(odataReq.Query, g.config.API.IdentitySearchMinLength) {
4738
if !ctxHasFullPerms {
4839
// for regular user the search term must have a minimum length
4940
logger.Debug().Interface("query", r.URL.Query()).Msgf("search with less than %d chars for a regular user", g.config.API.IdentitySearchMinLength)
5041
errorcode.AccessDenied.Render(w, r, http.StatusForbidden, "search term too short")
5142
return
5243
}
53-
54-
if !mfa.EnsureOrReject(r.Context(), w) {
44+
if !hasMFA {
5545
logger.Error().Str("path", r.URL.Path).Msg("MFA required but not satisfied")
46+
mfa.SetRequiredStatus(w)
5647
return
5748
}
5849
}
5950

60-
if !ctxHasFullPerms && (odataReq.Query.Filter != nil || odataReq.Query.Apply != nil || odataReq.Query.Expand != nil || odataReq.Query.Compute != nil) {
61-
// regular users can't use filter, apply, expand and compute
62-
logger.Debug().Interface("query", r.URL.Query()).Msg("forbidden query elements for a regular user")
63-
errorcode.AccessDenied.Render(w, r, http.StatusForbidden, "query has forbidden elements for regular users")
64-
return
51+
if !hasAcceptableQuery(odataReq.Query) {
52+
if !ctxHasFullPerms {
53+
// regular users can't use filter, apply, expand and compute
54+
logger.Debug().Interface("query", r.URL.Query()).Msg("forbidden query elements for a regular user")
55+
errorcode.AccessDenied.Render(w, r, http.StatusForbidden, "query has forbidden elements for regular users")
56+
return
57+
}
58+
59+
if !hasMFA {
60+
logger.Error().Str("path", r.URL.Path).Msg("MFA required but not satisfied")
61+
mfa.SetRequiredStatus(w)
62+
return
63+
}
6564
}
6665

6766
groups, err := g.identityBackend.GetGroups(r.Context(), odataReq)

services/graph/pkg/service/v0/users.go

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -222,48 +222,49 @@ func (g Graph) GetUsers(w http.ResponseWriter, r *http.Request) {
222222
}
223223

224224
ctxHasFullPerms := g.contextUserHasFullAccountPerms(r.Context())
225-
searchHasAcceptableLength := false
226-
if odataReq.Query != nil && odataReq.Query.Search != nil {
227-
minSearchLength := g.config.API.IdentitySearchMinLength
228-
if strings.HasPrefix(odataReq.Query.Search.RawValue, "\"") {
229-
// if search starts with double quotes then it must finish with double quotes
230-
// add +2 to the minimum search length in this case
231-
minSearchLength += 2
232-
}
233-
searchHasAcceptableLength = len(odataReq.Query.Search.RawValue) >= minSearchLength
234-
}
235-
if !searchHasAcceptableLength {
225+
hasMFA := mfa.Has(r.Context())
226+
if !hasAcceptableSearch(odataReq.Query, g.config.API.IdentitySearchMinLength) {
236227
if !ctxHasFullPerms {
237228
// for regular user the search term must have a minimum length
238229
logger.Debug().Interface("query", r.URL.Query()).Msgf("search with less than %d chars for a regular user", g.config.API.IdentitySearchMinLength)
239230
errorcode.AccessDenied.Render(w, r, http.StatusForbidden, "search term too short")
240231
return
241232
}
242-
if !mfa.EnsureOrReject(r.Context(), w) {
233+
if !hasMFA {
243234
logger.Error().Str("path", r.URL.Path).Msg("MFA required but not satisfied")
235+
mfa.SetRequiredStatus(w)
244236
return
245237
}
246238
}
247239

248-
if !ctxHasFullPerms && odataReq.Query.Filter != nil {
249-
// regular users are allowed to filter only by userType
250-
filter := odataReq.Query.Filter
251-
switch {
252-
case filter.Tree.Token.Type != godata.ExpressionTokenLogical:
253-
fallthrough
254-
case filter.Tree.Token.Value != "eq":
255-
fallthrough
256-
case filter.Tree.Children[0].Token.Value != "userType":
240+
if !hasAcceptableFilter(odataReq.Query) {
241+
if !ctxHasFullPerms {
242+
// regular users are allowed to filter only by userType
257243
logger.Debug().Interface("query", r.URL.Query()).Msg("forbidden filter for a regular user")
258244
errorcode.AccessDenied.Render(w, r, http.StatusForbidden, "filter has forbidden elements for regular users")
259245
return
260246
}
247+
248+
if !hasMFA {
249+
logger.Error().Str("path", r.URL.Path).Msg("MFA required but not satisfied")
250+
mfa.SetRequiredStatus(w)
251+
return
252+
}
261253
}
262-
if !ctxHasFullPerms && (odataReq.Query.Apply != nil || odataReq.Query.Expand != nil || odataReq.Query.Compute != nil) {
263-
// regular users can't use filter, apply, expand and compute
264-
logger.Debug().Interface("query", r.URL.Query()).Msg("forbidden query elements for a regular user")
265-
errorcode.AccessDenied.Render(w, r, http.StatusForbidden, "query has forbidden elements for regular users")
266-
return
254+
255+
if !hasAcceptableQuery(odataReq.Query) {
256+
if !ctxHasFullPerms {
257+
// regular users can't use filter, apply, expand and compute
258+
logger.Debug().Interface("query", r.URL.Query()).Msg("forbidden query elements for a regular user")
259+
errorcode.AccessDenied.Render(w, r, http.StatusForbidden, "query has forbidden elements for regular users")
260+
return
261+
}
262+
263+
if !hasMFA {
264+
logger.Error().Str("path", r.URL.Path).Msg("MFA required but not satisfied")
265+
mfa.SetRequiredStatus(w)
266+
return
267+
}
267268
}
268269

269270
logger.Debug().Interface("query", r.URL.Query()).Msg("calling get users on backend")

0 commit comments

Comments
 (0)