Some improvements

Author: lunny

routers/api/v1/api.go

@@ -235,54 +235,6 @@ func reqPackageAccess(accessMode perm.AccessMode) func(ctx *context.APIContext)
 	}
 }
 
-// if a token is being used for auth, we check that it contains the required scope
-// if a token is not being used, reqToken will enforce other sign in methods
-func tokenRequiresScopes(requiredScopeCategories ...auth_model.AccessTokenScopeCategory) func(ctx *context.APIContext) {
-	return func(ctx *context.APIContext) {
-		// no scope required
-		if len(requiredScopeCategories) == 0 {
-			return
-		}
-
-		// Need OAuth2 token to be present.
-		scope, scopeExists := ctx.Data["ApiTokenScope"].(auth_model.AccessTokenScope)
-		if ctx.Data["IsApiToken"] != true || !scopeExists {
-			return
-		}
-
-		// use the http method to determine the access level
-		requiredScopeLevel := auth_model.Read
-		if ctx.Req.Method == "POST" || ctx.Req.Method == "PUT" || ctx.Req.Method == "PATCH" || ctx.Req.Method == "DELETE" {
-			requiredScopeLevel = auth_model.Write
-		}
-
-		// get the required scope for the given access level and category
-		requiredScopes := auth_model.GetRequiredScopes(requiredScopeLevel, requiredScopeCategories...)
-		allow, err := scope.HasScope(requiredScopes...)
-		if err != nil {
-			ctx.Error(http.StatusForbidden, "tokenRequiresScope", "checking scope failed: "+err.Error())
-			return
-		}
-
-		if !allow {
-			ctx.Error(http.StatusForbidden, "tokenRequiresScope", fmt.Sprintf("token does not have at least one of required scope(s): %v", requiredScopes))
-			return
-		}
-
-		ctx.Data["requiredScopeCategories"] = requiredScopeCategories
-
-		// check if scope only applies to public resources
-		publicOnly, err := scope.PublicOnly()
-		if err != nil {
-			ctx.Error(http.StatusForbidden, "tokenRequiresScope", "parsing public resource scope failed: "+err.Error())
-			return
-		}
-
-		// assign to true so that those searching should only filter public repositories/users/organizations
-		ctx.PublicOnly = publicOnly
-	}
-}
-
 func checkTokenPublicOnly() func(ctx *context.APIContext) {
 	return func(ctx *context.APIContext) {
 		if !ctx.PublicOnly {
@@ -296,12 +248,16 @@ func checkTokenPublicOnly() func(ctx *context.APIContext) {
 
 		// public Only permission check
 		switch {
-		case auth_model.ContainsCategory(requiredScopeCategories, auth_model.AccessTokenScopeCategoryRepository),
-			auth_model.ContainsCategory(requiredScopeCategories, auth_model.AccessTokenScopeCategoryIssue):
+		case auth_model.ContainsCategory(requiredScopeCategories, auth_model.AccessTokenScopeCategoryRepository):
 			if ctx.Repo.Repository != nil && ctx.Repo.Repository.IsPrivate {
 				ctx.Error(http.StatusForbidden, "reqToken", "token scope is limited to public repos")
 				return
 			}
+		case auth_model.ContainsCategory(requiredScopeCategories, auth_model.AccessTokenScopeCategoryIssue):
+			if ctx.Repo.Repository != nil && ctx.Repo.Repository.IsPrivate {
+				ctx.Error(http.StatusForbidden, "reqToken", "token scope is limited to public issues")
+				return
+			}
 		case auth_model.ContainsCategory(requiredScopeCategories, auth_model.AccessTokenScopeCategoryOrganization):
 			if ctx.Org.Organization != nil && ctx.Org.Organization.Visibility != api.VisibleTypePublic {
 				ctx.Error(http.StatusForbidden, "reqToken", "token scope is limited to public orgs")
@@ -335,6 +291,54 @@ func checkTokenPublicOnly() func(ctx *context.APIContext) {
 	}
 }
 
+// if a token is being used for auth, we check that it contains the required scope
+// if a token is not being used, reqToken will enforce other sign in methods
+func tokenRequiresScopes(requiredScopeCategories ...auth_model.AccessTokenScopeCategory) func(ctx *context.APIContext) {
+	return func(ctx *context.APIContext) {
+		// no scope required
+		if len(requiredScopeCategories) == 0 {
+			return
+		}
+
+		// Need OAuth2 token to be present.
+		scope, scopeExists := ctx.Data["ApiTokenScope"].(auth_model.AccessTokenScope)
+		if ctx.Data["IsApiToken"] != true || !scopeExists {
+			return
+		}
+
+		// use the http method to determine the access level
+		requiredScopeLevel := auth_model.Read
+		if ctx.Req.Method == "POST" || ctx.Req.Method == "PUT" || ctx.Req.Method == "PATCH" || ctx.Req.Method == "DELETE" {
+			requiredScopeLevel = auth_model.Write
+		}
+
+		// get the required scope for the given access level and category
+		requiredScopes := auth_model.GetRequiredScopes(requiredScopeLevel, requiredScopeCategories...)
+		allow, err := scope.HasScope(requiredScopes...)
+		if err != nil {
+			ctx.Error(http.StatusForbidden, "tokenRequiresScope", "checking scope failed: "+err.Error())
+			return
+		}
+
+		if !allow {
+			ctx.Error(http.StatusForbidden, "tokenRequiresScope", fmt.Sprintf("token does not have at least one of required scope(s): %v", requiredScopes))
+			return
+		}
+
+		ctx.Data["requiredScopeCategories"] = requiredScopeCategories
+
+		// check if scope only applies to public resources
+		publicOnly, err := scope.PublicOnly()
+		if err != nil {
+			ctx.Error(http.StatusForbidden, "tokenRequiresScope", "parsing public resource scope failed: "+err.Error())
+			return
+		}
+
+		// assign to true so that those searching should only filter public repositories/users/organizations
+		ctx.PublicOnly = publicOnly
+	}
+}
+
 // Contexter middleware already checks token for user sign in process.
 func reqToken() func(ctx *context.APIContext) {
 	return func(ctx *context.APIContext) {
@@ -941,17 +945,17 @@ func Routes() *web.Router {
 		// Notifications (requires 'notifications' scope)
 		m.Group("/notifications", func() {
 			m.Combo("").
-				Get(notify.ListNotifications).
-				Put(notify.ReadNotifications)
-			m.Get("/new", notify.NewAvailable)
+				Get(reqToken(), notify.ListNotifications).
+				Put(reqToken(), notify.ReadNotifications)
+			m.Get("/new", reqToken(), notify.NewAvailable)
 			m.Combo("/threads/{id}").
-				Get(notify.GetThread).
-				Patch(notify.ReadThread)
-		}, reqToken(), tokenRequiresScopes(auth_model.AccessTokenScopeCategoryNotification))
+				Get(reqToken(), notify.GetThread).
+				Patch(reqToken(), notify.ReadThread)
+		}, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryNotification))
 
 		// Users (requires user scope)
 		m.Group("/users", func() {
-			m.Get("/search", reqExploreSignIn(), tokenRequiresScopes(auth_model.AccessTokenScopeCategoryUser), user.Search)
+			m.Get("/search", reqExploreSignIn(), user.Search)
 
 			m.Group("/{username}", func() {
 				m.Get("", reqExploreSignIn(), user.GetInfo)