Merge pull request #335 from snap-cloud/debug-wrong-passwords

cycomachead · web-flow · commit 8c782e572629 · 2023-05-14T18:06:59.000+02:00
Finish Bulk Account Creation
diff --git a/api.lua b/api.lua
@@ -90,8 +90,11 @@ app:match(api_route('my_user'), respond_to({
     DELETE = UserController.delete
 }))
 
+app:match('logout', respond_to({
+    GET = UserController.logout_get,
+}))
+
 app:match(api_route('logout'), respond_to({
-    GET = UserController.logout,
     POST = UserController.logout
 }))
 
diff --git a/app.lua b/app.lua
@@ -283,9 +283,13 @@ function app:handle_404()
 end
 
 function app:handle_error(err, trace)
-    local inspect = require('inspect')
-    print(inspect(err))
-    print(inspect(trace))
+    if config._name == 'development' then
+        debug_print(err, trace)
+        local msg = '<pre style="text-align: left; width: 80ch">'
+            .. err .. '<br>' .. trace .. '</pre>'
+        return errorResponse(msg, 500)
+    end
+
     local err_msg = exceptions.normalize_error(err)
     local user_info = exceptions.get_user_info(self.session)
     if config.sentry_dsn then
diff --git a/bin/deploy b/bin/deploy
@@ -10,19 +10,21 @@ echo "Deploying branch: $branch"
 # Deploy a copy of snapCloud
 pushd ~/snapCloud/ > /dev/null;
 
+git fetch
 git checkout $branch
-git pull origin $branch
+# git pull origin $branch
 # Always init, just incase we add a submodule.
 git submodule init
 git submodule update --recursive --remote
 
+# TODO: Clean this up...
 pushd ~/snapCloud/bin/;
-
 ./renew-certs
 ./deploy-certs
+popd;
+
 # ./update-snap
 
-popd;
 
 echo "Updating Dependencies:"
 sudo luarocks install --only-deps snapcloud-dev-0.rockspec
diff --git a/controllers/user.lua b/controllers/user.lua
@@ -156,6 +156,7 @@ UserController = {
             if self.queried_user.verified then
                 return okResponse('User ' .. self.queried_user.username
                         .. ' logged in')
+            -- TODO: Handle first-time student account logins.
             else
                 return jsonResponse({
                     title = 'Verify your account',
@@ -171,13 +172,19 @@ UserController = {
             return self:build_url('index')
         end
     end),
+    logout_get = capture_errors(function (self)
+        self.session.username = ''
+        self.session.user_id = nil
+        self.cookies.persist_session = 'false'
+        return { redirect_to = self:build_url('/') }
+    end),
     logout = capture_errors(function (self)
         self.session.username = ''
         self.session.user_id = nil
         self.cookies.persist_session = 'false'
-        return jsonResponse(
-            { redirect = self.params.redirect or self:build_url('index') }
-        )
+        return jsonResponse({
+            redirect = (self.params.redirect or self:build_url('/'))
+        })
     end),
     change_email = capture_errors(function (self)
         assert_logged_in(self)
@@ -187,6 +194,8 @@ UserController = {
         if self.queried_user then
             -- we're trying to change someone else's email
             assert_min_role(self, 'moderator')
+        elseif user:is_student() then
+            yield_error(err.student_cannot_change_email)
         elseif (user.password ~=
                 hash_password(self.params.password, user.salt)) then
             yield_error(err.wrong_password)
@@ -227,7 +236,7 @@ UserController = {
             local minutes = db.select(
                 'extract(minutes from (now()::timestamp - ?::timestamp))',
                 token.created)[1].date_part
-            if minutes < 15 then
+            if minutes and minutes < 15 then
                 yield_error(err.too_many_password_resets)
             end
         end
@@ -490,20 +499,21 @@ UserController = {
                 end
             end
         end
+        local salt, password, result
         for _, user in pairs(users) do
-            user.username = util.trim(tostring(user.username))
-            user.password = util.trim(tostring(user.password))
-            user.email = user.email or self.current_user.email
-            -- TODO: This doesn't reveal which record has an invalid value...
-            validate.assert_valid(user, Users.validations)
-
-            user.created = db.format_date()
-            user.salt = secure_salt()
-            user.password = hash_password(user.password, user.salt)
-            user.verified = true
-            user.role = 'student'
-            user.creator_id = self.current_user.id
-            local result = Users:create(user)
+            salt = secure_salt()
+            password = util.trim(tostring(user.password))
+            result = Users:create({
+                created = db.format_date(),
+                username = util.trim(tostring(user.username)),
+                salt = salt,
+                password = hash_password(hash_password(password, ''), salt),
+                email = (user.email or self.current_user.email),
+                verified = true,
+                role = 'student',
+                creator_id = self.current_user.id
+            })
+
             if not result then
                 db.query('ROLLBACK;')
                 return errorResponse(
diff --git a/lib/util.lua b/lib/util.lua
@@ -27,5 +27,5 @@ local function capitalize(str)
 end
 
 return {
-  capitalize
+  capitalize = capitalize
 }
diff --git a/static/style/classes.css b/static/style/classes.css
@@ -167,3 +167,11 @@ pre.in-place {
 img.emoji {
     height: 1em;
 }
+
+.code-example {
+    border: 1px solid;
+    border-radius: 4px;
+    margin: 1em;
+    padding: 0.5em;
+    background: rgb(248, 248, 255);
+}
diff --git a/validation.lua b/validation.lua
@@ -118,6 +118,10 @@ err = {
     teacher_account_required = {
         msg = 'You must be verified as a teacher to perform this action.',
         status = 403
+    },
+    student_cannot_change_email = {
+        msg = 'Student account email addresses cannot be changed. Please contact your instructor for help.',
+        status = 403
     }
 }
 
diff --git a/views/bulk.etlua b/views/bulk.etlua
@@ -1,49 +1,70 @@
 <script src="/static/js/papaparse.min.js"></script>
+
 <h1><%- locale.get('bulk_tile') %></h1>
 <div class="bulk">
     <p><%- locale.get('bulk_text') %></p>
-    <label for="csv">Upload a CSV file:</label>
-    <input type="file" class="csv" name="csv">
-    <p>
-        <input type="checkbox" class="add-collection" name="add_collection" 
-            onchange="
-                document.querySelector('div.collection').hidden =
-                    !document.querySelector('div.collection').hidden;
-        ">
-        <label for="add_collection"
-            ><%- locale.get('bulk_make_collection') %></label>
-        <div class="collection" hidden>
-            <strong><%- locale.get('collection_name') %></strong>
-            <input name="collection_name" class="collection_name"></input>
-        </div>
-    </p>
-    <button class="pure-button" onclick="
-        var div = document.querySelector('div.bulk'),
-            collection_name =
-                div.querySelector('input.add-collection').checked ?
-                    div.querySelector('.collection_name').value :
-                    null,
-            file = div.querySelector('.csv').files[0];
-        Papa.parse(
-            file,
-            { 
-                skipEmptyLines: true,
-                header: true,
-                complete: (results) => {
-                    cloud.post(
-                        '/users/create_learners',
+    <div>
+        <p>Example:</p>
+        <pre class="code-example">
+username,password
+myclass-student1,correct-horse-battery-staple
+myclass-student2,sentences-make-good-passowrds</pre>
+    </div>
+    <div class="pure-form">
+        <br>
+        <label for="csv"><strong>Upload a CSV file:</strong>
+            <input id="csv" type="file" class="csv" name="csv">
+        </label>
+        <br><br>
+        <details>
+            <summary>Alternatively, paste a CSV File</summary>
+            <br>
+            <label for="csv_txt">CSV Contents</label>
+            <br>
+            <textarea id="csv_text" rows="5" name="csv_text" class="pure-input-1-2"
+                style="font-family: monospace"></textarea>
+        </details>
+        <p>
+            <label for="add_collection" class="pure-checkbox">
+                <input id="add_collection" type="checkbox" class="add-collection" name="add_collection"
+                    onchange="document.querySelector('div.collection').hidden =
+                            !document.querySelector('div.collection').hidden;">
+                <%- locale.get('bulk_make_collection') %>
+            </label>
+            <div class="collection" hidden>
+                <label for="collection_name">
+                    <strong><%- locale.get('collection_name') %></strong>
+                </label>
+                <input id="collection_name" name="collection_name" class="collection_name" />
+            </div>
+        </p>
+        <button class="pure-button" onclick="
+            var div = document.querySelector('div.bulk'),
+                collection_name =
+                    div.querySelector('input.add-collection').checked ?
+                        div.querySelector('.collection_name').value :
                         null,
-                        {
-                            collection_name: collection_name,
-                            users: results.data
+                file = div.querySelector('.csv').files[0],
+                csv_text = document.querySelector('#csv_text').value,
+                csv_data = csv_text || file;
+            Papa.parse(
+                csv_data,
+                {
+                    skipEmptyLines: true,
+                    header: true,
+                    complete: (results) => {
+                        let post_body = { users: results.data };
+                        if (collection_name) {
+                            post_body.collection_name = collection_name;
                         }
-                    );
+                        cloud.post('/users/create_learners', null, post_body);
+                    }
                 }
-            }
-        );
-    "><%- locale.get('bulk_create') %></button>
-    <script>
-        document.querySelector('div.collection').hidden = 
-            !document.querySelector('input.add-collection').checked;
-    </script>
+            );
+        "><%- locale.get('bulk_create') %></button>
+        <script>
+            document.querySelector('div.collection').hidden =
+                !document.querySelector('input.add-collection').checked;
+        </script>
+    </div>
 </div>
diff --git a/views/profile.etlua b/views/profile.etlua
@@ -21,8 +21,10 @@
     <div class="pure-u-1-2 buttons">
         <a class="pure-button"
             href="change_password"><%- locale.get('change_my_password') %></a>
+        <% if not user:is_student() then %>
         <a class="pure-button"
             href="change_email"><%- locale.get('change_my_email') %></a>
+        <% end %>
         <a class="pure-button pure-button-warning" onclick="
             confirmDeleteMyself()"><%- locale.get('delete_my_user') %></a>
     </div>
diff --git a/views/user_admin.etlua b/views/user_admin.etlua
@@ -28,6 +28,7 @@ render(
                     { label = 'Reviewer', value = 'reviewer' },
                     { label = 'Moderator', value = 'moderator' },
                     { label = 'Administrator', value = 'admin' },
+                    { label = 'Student', value = 'student' },
                     { label = 'Banned', value = 'banned' },
                 }
             }

Original file line number	Diff line number	Diff line change
`@@ -27,5 +27,5 @@ local function capitalize(str)`
`27`	`27`	`end`
`28`	`28`
`29`	`29`	`return {`
`30`		`- capitalize`
	`30`	`+ capitalize = capitalize`
`31`	`31`	`}`
Original file line number	Diff line number	Diff line change
`@@ -118,6 +118,10 @@ err = {`
`118`	`118`	`teacher_account_required = {`
`119`	`119`	`msg = 'You must be verified as a teacher to perform this action.',`
`120`	`120`	`status = 403`
	`121`	`+ },`
	`122`	`+ student_cannot_change_email = {`
	`123`	`+ msg = 'Student account email addresses cannot be changed. Please contact your instructor for help.',`
	`124`	`+ status = 403`
`121`	`125`	`}`
`122`	`126`	`}`
`123`	`127`
Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,7 @@ render(`
`28`	`28`	`{ label = 'Reviewer', value = 'reviewer' },`
`29`	`29`	`{ label = 'Moderator', value = 'moderator' },`
`30`	`30`	`{ label = 'Administrator', value = 'admin' },`
	`31`	`+ { label = 'Student', value = 'student' },`
`31`	`32`	`{ label = 'Banned', value = 'banned' },`
`32`	`33`	`}`
`33`	`34`	`}`