Skip to content

Commit

Permalink
dashboard/app: introduce authorized public clients
Browse files Browse the repository at this point in the history
Let's disable throttling for all authorized clients.
The only diff between the authorized and non-authorized public client is a throttling.
  • Loading branch information
tarasmadan committed Oct 15, 2024
1 parent 084d817 commit 0f1ec4d
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 36 deletions.
58 changes: 38 additions & 20 deletions dashboard/app/access.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,27 +46,45 @@ func checkAccessLevel(c context.Context, r *http.Request, level AccessLevel) err
return ErrAccess
}

// AuthDomain is broken in AppEngine tests.
var isBrokenAuthDomainInTest = false

func emailInAuthDomains(email string, authDomains []string) bool {
for _, authDomain := range authDomains {
if strings.HasSuffix(email, authDomain) {
func isAuthorizedUserDomain(c context.Context, u *user.User) bool {
if u == nil {
return false
}
for _, authDomain := range getConfig(c).AuthUserDomains {
if strings.HasSuffix(u.Email, authDomain) {
return true
}
}
return false
}

func isAuthorizedPublicEmail(c context.Context, u *user.User) bool {
if u == nil {
return false
}
for _, authEmail := range getConfig(c).AuthPublicEmails {
if u.Email == authEmail {
return true
}
}
return false
}

func currentUser(c context.Context, r *http.Request) *user.User {
func isAuthorized(c context.Context) bool {
u := user.Current(c)
if u != nil {
return u
return isAuthorizedUserDomain(c, u) || isAuthorizedPublicEmail(c, u)
}

func currentUser(c context.Context) *user.User {
u := user.Current(c)
if u == nil {
// Let's ignore err here. In case of the wrong token we'll return nil here (it means AccessPublic).
// Bad or expired tokens will also enable throttling and make the authorization problem visible.
u, _ = user.CurrentOAuth(c, "https://www.googleapis.com/auth/userinfo.email")
}
if u == nil || u.AuthDomain != "gmail.com" || u.FederatedIdentity != "" || u.FederatedProvider != "" {
u = nil
}
// Let's ignore err here. In case of the wrong token we'll return nil here (it means AccessPublic).
// Bad or expired tokens will also enable throttling and make the authorization problem visible.
u, _ = user.CurrentOAuth(c, "https://www.googleapis.com/auth/userinfo.email")
return u
}

Expand All @@ -78,7 +96,11 @@ func currentUser(c context.Context, r *http.Request) *user.User {
// OAuth2 token is expected to be present in "Authorization" header.
// Example: "Authorization: Bearer $(gcloud auth print-access-token)".
func accessLevel(c context.Context, r *http.Request) AccessLevel {
if user.IsAdmin(c) {
u := currentUser(c)
if u == nil {
return AccessPublic
}
if u.Admin && r != nil {
switch r.FormValue("access") {
case "public":
return AccessPublic
Expand All @@ -87,14 +109,10 @@ func accessLevel(c context.Context, r *http.Request) AccessLevel {
}
return AccessAdmin
}
u := currentUser(c, r)
if u == nil ||
// Devappserver does not pass AuthDomain.
u.AuthDomain != "gmail.com" && !isBrokenAuthDomainInTest ||
!emailInAuthDomains(u.Email, getConfig(c).AuthDomains) {
return AccessPublic
if isAuthorizedUserDomain(c, u) {
return AccessUser
}
return AccessUser
return AccessPublic
}

func checkTextAccess(c context.Context, r *http.Request, tag string, id int64) (*Bug, *Crash, error) {
Expand Down
87 changes: 87 additions & 0 deletions dashboard/app/access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/google/syzkaller/dashboard/dashapi"
"github.com/stretchr/testify/assert"
"google.golang.org/appengine/v2/aetest"
"google.golang.org/appengine/v2/user"
)

Expand Down Expand Up @@ -429,3 +430,89 @@ func TestAccess(t *testing.T) {
}
}
}

const (
BadAuthDomain = iota
Regular
Authenticated
AuthorizedAccessPublic
AuthorizedUser
AuthorizedAdmin
)

func makeUser(t int) *user.User {
u := &user.User{
AuthDomain: "gmail.com",
Admin: false,
FederatedIdentity: "",
FederatedProvider: "",
}
switch t {
case BadAuthDomain:
u.Email = "[email protected]"
u.AuthDomain = "public.com"
case Regular:
u = nil
case Authenticated:
u.Email = "[email protected]"
case AuthorizedAccessPublic:
u.Email = "[email protected]"
case AuthorizedUser:
u.Email = "[email protected]"
case AuthorizedAdmin:
u.Email ="[email protected]"

Check failure on line 463 in dashboard/app/access_test.go

View workflow job for this annotation

GitHub Actions / build

File is not `gofmt`-ed with `-s` (gofmt)
u.Admin = true
}
return u
}

func TestAuthorization(t *testing.T) {
c := NewCtx(t)
defer c.Close()

// BadAuthDomain gives no access.
assert.False(t, isAuthorizedUserDomain(c.ctx, makeUser(BadAuthDomain)))
assert.False(t, isAuthorizedPublicEmail(c.ctx, makeUser(BadAuthDomain)))

// Authentication gives nothing too.
assert.False(t, isAuthorizedPublicEmail(c.ctx, makeUser(Regular)))
assert.False(t, isAuthorizedUserDomain(c.ctx, makeUser(Regular)))

assert.False(t, isAuthorizedUserDomain(c.ctx, makeUser(Authenticated)))
assert.False(t, isAuthorizedPublicEmail(c.ctx, makeUser(Authenticated)))

// Authenticated + allowlisted users access w/o throttling.
assert.False(t, isAuthorizedUserDomain(c.ctx, makeUser(AuthorizedAccessPublic)))
assert.True(t, isAuthorizedPublicEmail(c.ctx, makeUser(AuthorizedAccessPublic)))
assert.True(t, isAuthorized(c.ctx))

// AccessUser gives evetything except admin rights.
assert.True(t, isAuthorizedUserDomain(c.ctx, makeUser(AuthorizedUser)))
assert.True(t, isAuthorizedPublicEmail(c.ctx, makeUser(AuthorizedUser)))
}

func TestAccessLevel(t *testing.T) {
c := NewCtx(t)
defer c.Close()
req, err := c.inst.NewRequest("GET", "", nil)
assert.NoError(t, err)

aetest.Login(makeUser(BadAuthDomain), req)
assert.Equal(t, AccessPublic, accessLevel(c.ctx, req))

aetest.Login(makeUser(Regular), req)
assert.Equal(t, AccessPublic, accessLevel(c.ctx, req))

aetest.Login(makeUser(Authenticated), req)
assert.Equal(t, AccessPublic, accessLevel(c.ctx, req))

aetest.Login(makeUser(AuthorizedAccessPublic), req)
assert.Equal(t, AccessPublic, accessLevel(c.ctx, req))

aetest.Login(makeUser(AuthorizedUser), req)
assert.Equal(t, AccessUser, accessLevel(c.ctx, req))

aetest.Login(makeUser(AuthorizedAdmin), req)
assert.Equal(t, AccessAdmin, accessLevel(c.ctx, req))

}

Check failure on line 518 in dashboard/app/access_test.go

View workflow job for this annotation

GitHub Actions / build

unnecessary trailing newline (whitespace)
6 changes: 3 additions & 3 deletions dashboard/app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ func init() {
os.Setenv("GAE_MODULE_VERSION", "1")
os.Setenv("GAE_MINOR_VERSION", "1")

isBrokenAuthDomainInTest = true
obsoleteWhatWontBeFixBisected = true
notifyAboutUnsuccessfulBisections = true
ensureConfigImmutability = true
Expand All @@ -41,8 +40,9 @@ func init() {

// Config used in tests.
var testConfig = &GlobalConfig{
AccessLevel: AccessPublic,
AuthDomains: []string{"@syzkaller.com"},
AccessLevel: AccessPublic,
AuthUserDomains: []string{"@syzkaller.com"},
AuthPublicEmails: []string{makeUser(AuthorizedUser).Email},
Clients: map[string]string{
"reporting": "reportingkeyreportingkeyreportingkey",
},
Expand Down
17 changes: 14 additions & 3 deletions dashboard/app/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ import (
type GlobalConfig struct {
// Min access levels specified hierarchically throughout the config.
AccessLevel AccessLevel
// Email suffixes of authorized users (e.g. []string{"@foo.com","@bar.org"}).
AuthDomains []string
// AuthUserDomains is a list of email prefixes authorized for UserAccess (e.g. []string{"@foo.com","@bar.org"}).
AuthUserDomains []string
// AuthPublicEmails is a list of emails authorized for PublicAccess w/o throttling.
AuthPublicEmails []string
// Google Analytics Tracking ID.
AnalyticsTrackingID string
// URL prefix of source coverage reports.
Expand Down Expand Up @@ -455,7 +457,8 @@ func checkConfig(cfg *GlobalConfig) {
checkNamespace(ns, cfg, namespaces, clientNames)
}
checkDiscussionEmails(cfg.DiscussionEmails)
checkAuthDomains(cfg.AuthDomains)
checkAuthDomains(cfg.AuthUserDomains)
checkAuthUsers(cfg.AuthPublicEmails)
}

func checkAuthDomains(list []string) {
Expand All @@ -466,6 +469,14 @@ func checkAuthDomains(list []string) {
}
}

func checkAuthUsers(list []string) {
for _, email := range list {
if strings.Count(email, "@") != 1 {
panic(fmt.Sprintf("%s is not an authorized user e-mail (no @ found)", email))
}
}
}

func checkDiscussionEmails(list []DiscussionEmailConfig) {
dup := map[string]struct{}{}
for _, item := range list {
Expand Down
2 changes: 1 addition & 1 deletion dashboard/app/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func handleContext(fn contextHandler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
c := appengine.NewContext(r)
c = context.WithValue(c, &currentURLKey, r.URL.RequestURI())
if accessLevel(c, r) == AccessPublic {
if !isAuthorized(c) {
if !throttleRequest(c, w, r) {
return
}
Expand Down
16 changes: 7 additions & 9 deletions dashboard/app/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import (
db "google.golang.org/appengine/v2/datastore"
"google.golang.org/appengine/v2/log"
aemail "google.golang.org/appengine/v2/mail"
"google.golang.org/appengine/v2/user"
)

type Ctx struct {
Expand Down Expand Up @@ -341,14 +340,13 @@ func (c *Ctx) httpRequest(method, url, body, contentType string,
}
r = registerRequest(r, c)
r = r.WithContext(c.transformContext(r.Context()))
if access == AccessAdmin || access == AccessUser {
user := &user.User{
Email: "[email protected]",
AuthDomain: "gmail.com",
}
if access == AccessAdmin {
user.Admin = true
}
user := makeUser(Regular)
if access == AccessAdmin {
user = makeUser(AuthorizedAdmin)
} else if access == AccessUser {
user = makeUser(AuthorizedUser)
}
if user != nil {
aetest.Login(user, r)
}
w := httptest.NewRecorder()
Expand Down

0 comments on commit 0f1ec4d

Please sign in to comment.