Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

admin: Add log out button #870

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BBaoVanC
Copy link
Contributor

@BBaoVanC BBaoVanC commented May 8, 2022

Fixes #552

Adds a logout button to the header, in the top right.

Tasks:

  • CHANGES.rst

image

Copy link
Member

@ix5 ix5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See inline comments

Edit 2: Whoops, I think that'd delete all current site cookies. Need a better way.

Edit: My idea, not complete:

diff --git a/isso/css/admin.css b/isso/css/admin.css
index 3f694c3..fd2bbb5 100644
--- a/isso/css/admin.css
+++ b/isso/css/admin.css
@@ -30,16 +30,25 @@ input {
 }
 .header header {
     display: block;
-    float: left;
     font-weight: normal;
     margin-right: 16.0363%;
-    width: 41.9818%;
 }
 .header header .logo {
-    float: left;
     max-height: 60px;
     padding-right: 12px;
 }
+.header header .title {
+    display: inline;
+}
+.header header .title a {
+    display: inline-block;
+    width: 41.9818%;
+}
+.header header .logout {
+    float: right;
+    margin-top: 0.8em;
+    line-height: 1.6em;
+}
 .header header h1 {
     font-size: 1.55em;
     margin-bottom: 0.3em;
diff --git a/isso/js/admin.js b/isso/js/admin.js
index 5b1e508..8d48167 100644
--- a/isso/js/admin.js
+++ b/isso/js/admin.js
@@ -124,4 +124,11 @@ function send_edit(com_id, hash, isso_host_script) {
     edit(com_id, hash, author, email, website, comment, isso_host_script);
     stop_edit(com_id, true);
 }
-
+function log_out() {
+    // Delete cookie
+    var cookie_name = "admin-session";
+    var domain = window.location.hostname;
+    document.cookie = cookie_name + "=; Max-Age=0; domain=" + domain + "; path=/";
+    // Redirect back to /admin login page
+    window.location.href = "admin";
+}
diff --git a/isso/templates/admin.html b/isso/templates/admin.html
index 3332c5b..10dd6f9 100644
--- a/isso/templates/admin.html
+++ b/isso/templates/admin.html
@@ -16,6 +16,7 @@
             <h2>Administration</h2>
           </a>
         </div>
+        <a id="logout" class="logout label" onClick="javascript:log_out()">Log out</a>
       </header>
     </div>
     <div class="outer">

isso/views/comments.py Outdated Show resolved Hide resolved
isso/views/comments.py Outdated Show resolved Hide resolved
@ix5 ix5 added server (Python) server code client (Javascript) client code and CSS feature labels Jun 4, 2022
@ix5 ix5 added this to the 0.14 milestone Jun 4, 2022
@ix5 ix5 mentioned this pull request Jun 4, 2022
@BBaoVanC BBaoVanC force-pushed the admin-logout-button branch 2 times, most recently from 390f74a to 8ea93d7 Compare June 12, 2022 19:06
@BBaoVanC
Copy link
Contributor Author

Wow, the admin CSS is a horrible mess right now. is it better to just do as little change here (like I already have) so it works, and then make another pull request to clean up the CSS?

@ix5
Copy link
Member

ix5 commented Jun 13, 2022

Wow, the admin CSS is a horrible mess right now. is it better to just do as little change here (like I already have) so it works, and then make another pull request to clean up the CSS?

Re: Admin CSS: Yes, please open another PR for that and keep this PR focused.

Re: Clearing the cookies: I'm not quite sure whether we need to match SameSite and Secure attributes from server when clearing cookies. Also, IMO we can use a bit more "modern" JS and modern Interfaces such as CookieStore in the admin (just keep the client JS as backward-compatible as possible).

@ix5
Copy link
Member

ix5 commented Jul 6, 2022

@BBaoVanC would you mind applying the PR feedback? I know it's a bit much to ask, but we should at least try to keep track of SameSite and Secure stuff, I think.

@jelmer
Copy link
Member

jelmer commented Aug 4, 2023

Please rebase this PR after the force-push.

Copy link
Member

@jelmer jelmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase

@BBaoVanC
Copy link
Contributor Author

Please rebase

Done

@BBaoVanC
Copy link
Contributor Author

BBaoVanC commented Nov 5, 2023

Is there anything else this PR needs? I believe when I last looked at it I had already applied @ix5's changes.

@ix5 ix5 modified the milestones: 0.14, 0.13.1 May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client (Javascript) client code and CSS feature server (Python) server code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Log out of admin interface
3 participants