From 17ef1e796a89205aab4781305ae963bfd30d7002 Mon Sep 17 00:00:00 2001
From: Pavel Krymets <pavel@krymets.com>
Date: Thu, 23 Jan 2020 11:50:26 -0800
Subject: [PATCH] Fix Unauthorised redirect (#349)

---
 .../Account/ConfigureOrganizationPolicy.cs    |  2 +-
 .../Controllers/AccountController.cs          |  2 +-
 .../APIView/APIViewWeb/Pages/Login.cshtml     | 13 ++++++++++
 .../APIView/APIViewWeb/Pages/Login.cshtml.cs  | 20 ++++++++++++++++
 .../APIViewWeb/Pages/Unauthorized.cshtml      | 10 ++++----
 .../APIViewWeb/Pages/Unauthorized.cshtml.cs   | 24 +++++++++++--------
 src/dotnet/APIView/APIViewWeb/Startup.cs      | 10 ++++++--
 7 files changed, 62 insertions(+), 19 deletions(-)
 create mode 100644 src/dotnet/APIView/APIViewWeb/Pages/Login.cshtml
 create mode 100644 src/dotnet/APIView/APIViewWeb/Pages/Login.cshtml.cs

diff --git a/src/dotnet/APIView/APIViewWeb/Account/ConfigureOrganizationPolicy.cs b/src/dotnet/APIView/APIViewWeb/Account/ConfigureOrganizationPolicy.cs
index dbff768e65..e5d9d7c17c 100644
--- a/src/dotnet/APIView/APIViewWeb/Account/ConfigureOrganizationPolicy.cs
+++ b/src/dotnet/APIView/APIViewWeb/Account/ConfigureOrganizationPolicy.cs
@@ -17,7 +17,7 @@ public ConfigureOrganizationPolicy(IOptions<OrganizationOptions> options)
 
         public void Configure(AuthorizationOptions options)
         {
-            options.AddPolicy("RequireOrganization", policy =>
+            options.AddPolicy(Startup.RequireOrganizationPolicy, policy =>
             {
                 policy.AddRequirements(new OrganizationRequirement(_options.Value.RequiredOrganization));
             });
diff --git a/src/dotnet/APIView/APIViewWeb/Controllers/AccountController.cs b/src/dotnet/APIView/APIViewWeb/Controllers/AccountController.cs
index 0b317056b1..38e5a4ba79 100644
--- a/src/dotnet/APIView/APIViewWeb/Controllers/AccountController.cs
+++ b/src/dotnet/APIView/APIViewWeb/Controllers/AccountController.cs
@@ -23,7 +23,7 @@ public async Task<IActionResult> Login(string returnUrl = "/")
         public async Task<IActionResult> Logout()
         {
             await HttpContext.SignOutAsync();
-            return RedirectToPage("/Unauthorized");
+            return RedirectToPage("/Login");
         }
     }
 }
diff --git a/src/dotnet/APIView/APIViewWeb/Pages/Login.cshtml b/src/dotnet/APIView/APIViewWeb/Pages/Login.cshtml
new file mode 100644
index 0000000000..8b8becc17d
--- /dev/null
+++ b/src/dotnet/APIView/APIViewWeb/Pages/Login.cshtml
@@ -0,0 +1,13 @@
+@page
+@model LoginModel
+@attribute [Microsoft.AspNetCore.Authorization.AllowAnonymous]
+@{
+    ViewData["Title"] = "Login";
+}
+
+<p>Please login using your GitHub account</p>
+
+<a class="login-button btn btn-outline-dark" asp-action="Login" asp-controller="Account" asp-route-returnUrl="@Model.ReturnUrl">
+    <img height="30" width="30" src="https://github.githubassets.com/images/modules/logos_page/GitHub-Mark.png" />
+    Login with GitHub
+</a>
\ No newline at end of file
diff --git a/src/dotnet/APIView/APIViewWeb/Pages/Login.cshtml.cs b/src/dotnet/APIView/APIViewWeb/Pages/Login.cshtml.cs
new file mode 100644
index 0000000000..05d9bb9b1d
--- /dev/null
+++ b/src/dotnet/APIView/APIViewWeb/Pages/Login.cshtml.cs
@@ -0,0 +1,20 @@
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.Mvc.RazorPages;
+
+namespace APIViewWeb.Pages
+{
+    public class LoginModel : PageModel
+    {
+        [BindProperty(SupportsGet = true, Name = "returnurl")]
+        public string ReturnUrl { get; set; } = "/";
+
+        public IActionResult OnGetAsync()
+        {
+            if (User.Identity.IsAuthenticated)
+                return Redirect(ReturnUrl);
+
+            return Page();
+        }
+    }
+}
\ No newline at end of file
diff --git a/src/dotnet/APIView/APIViewWeb/Pages/Unauthorized.cshtml b/src/dotnet/APIView/APIViewWeb/Pages/Unauthorized.cshtml
index 3c9e8ab096..c502d54be9 100644
--- a/src/dotnet/APIView/APIViewWeb/Pages/Unauthorized.cshtml
+++ b/src/dotnet/APIView/APIViewWeb/Pages/Unauthorized.cshtml
@@ -1,20 +1,20 @@
 @page
 @model APIViewWeb.Pages.UnauthorizedModel
-@attribute [Microsoft.AspNetCore.Authorization.AllowAnonymous]
+@attribute [Microsoft.AspNetCore.Authorization.Authorize]
 @{
     ViewData["Title"] = "Unauthorized";
     var names = string.Join(" or ", Model.Options.RequiredOrganization);
 }
 
-<p>You're currently not logged into a GitHub account or your GitHub account is not publicly affiliated with @names organizations.</p>
+<p>Your GitHub account is not publicly affiliated with @names organizations.</p>
 <p>
     In order to use the site, please sign into another account - or,
-    request to join one of the following organizations:
+    ensure <b>public</b> membership in one of the following organizations:
     <ul>
         @foreach (var organization in Model.Options.RequiredOrganization)
         {
             <li>
-                <a href="https://github.com/@organization">@organization</a>
+                <a href="https://github.com/orgs/@organization/people?utf8=✓&query=@(User.GetGitHubLogin())#org-members-table">@organization</a>
             </li>
         }
     </ul>
@@ -26,5 +26,5 @@
 
 <a class="login-button btn btn-outline-dark" asp-action="Login" asp-controller="Account" asp-route-returnUrl="@Model.ReturnUrl">
     <img height="30" width="30" src="https://github.githubassets.com/images/modules/logos_page/GitHub-Mark.png" />
-    Login with GitHub
+    Refresh GitHub login
 </a>
\ No newline at end of file
diff --git a/src/dotnet/APIView/APIViewWeb/Pages/Unauthorized.cshtml.cs b/src/dotnet/APIView/APIViewWeb/Pages/Unauthorized.cshtml.cs
index dc23710cf5..4279450cc1 100644
--- a/src/dotnet/APIView/APIViewWeb/Pages/Unauthorized.cshtml.cs
+++ b/src/dotnet/APIView/APIViewWeb/Pages/Unauthorized.cshtml.cs
@@ -1,4 +1,5 @@
-using Microsoft.AspNetCore.Authorization;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.RazorPages;
 using Microsoft.Extensions.Options;
@@ -7,23 +8,26 @@ namespace APIViewWeb.Pages
 {
     public class UnauthorizedModel : PageModel
     {
+        private readonly IAuthorizationService _authorizationService;
         public OrganizationOptions Options { get; }
 
-        [BindProperty(SupportsGet = true)]
-        public string ReturnUrl { get; private set; }
+        [BindProperty(SupportsGet = true, Name = "returnurl")]
+        public string ReturnUrl { get; set; } = "/";
 
-        public UnauthorizedModel(IOptions<OrganizationOptions> options)
+        public UnauthorizedModel(IOptions<OrganizationOptions> options, IAuthorizationService authorizationService)
         {
+            _authorizationService = authorizationService;
             Options = options.Value;
         }
 
-        public IActionResult OnGet()
+        public async Task<IActionResult> OnGetAsync()
         {
-            if (User.Identity.IsAuthenticated)
-            {
-                return RedirectToPage("./Assemblies/Index");
-            }
-            ReturnUrl = Request.Query["returnurl"];
+            var authorizationResult =
+                await _authorizationService.AuthorizeAsync(User, null, Startup.RequireOrganizationPolicy);
+
+            if (authorizationResult.Succeeded)
+                return Redirect(ReturnUrl);
+
             return Page();
         }
     }
diff --git a/src/dotnet/APIView/APIViewWeb/Startup.cs b/src/dotnet/APIView/APIViewWeb/Startup.cs
index 7f87d27a97..ff1d516e99 100644
--- a/src/dotnet/APIView/APIViewWeb/Startup.cs
+++ b/src/dotnet/APIView/APIViewWeb/Startup.cs
@@ -26,6 +26,8 @@ namespace APIViewWeb
 {
     public class Startup
     {
+        public static string RequireOrganizationPolicy = "RequireOrganization";
+
         public static string VersionHash { get; set; }
 
         static Startup()
@@ -64,7 +66,7 @@ public void ConfigureServices(IServiceCollection services)
 
             services.AddRazorPages(options =>
             {
-                options.Conventions.AuthorizeFolder("/Assemblies", "RequireOrganization");
+                options.Conventions.AuthorizeFolder("/Assemblies", RequireOrganizationPolicy);
                 options.Conventions.AddPageRoute("/Assemblies/Index", "");
             });
 
@@ -87,7 +89,11 @@ public void ConfigureServices(IServiceCollection services)
                     options.DefaultSignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
                     options.DefaultChallengeScheme = CookieAuthenticationDefaults.AuthenticationScheme;
                 })
-                .AddCookie(options => options.LoginPath = options.AccessDeniedPath = "/Unauthorized")
+                .AddCookie(options =>
+                {
+                    options.LoginPath = "/Login";
+                    options.AccessDeniedPath = "/Unauthorized";
+                })
                 .AddOAuth("GitHub", options =>
                 {
                     options.ClientId = Configuration["Github:ClientId"];