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.

Sign up for GitHub

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

Remove token after used or when expired #106

Closed
wants to merge 2 commits into from
Closed

Remove token after used or when expired #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
268613a
Remove token after used or when expired
alemela Jun 30, 2014
80ff7d3
Some fixes and better error response
alemela Jul 7, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions registry/backend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ var utils = require("./utils");

var MATRICOLA = /^[0-9]{6}$/;
var TOKEN = /^[A-Fa-f0-9]{40}$/;
var MAYBE_EMPTY_TOKEN = /^(|[A-Fa-f0-9]{40})$/;
var PWDHASH = /^[A-Fa-f0-9]{32}$/;
var URL = /^(|http(|s)\:\/\/[A-Za-z0-9\.\-\_\%\?\&\=\/]+)$/;

Expand Down Expand Up @@ -111,12 +112,32 @@ exports.saveUsers = function (matricola, hash, callback) {
return;
}
console.log("backend: password stored for %s", matricola);
callback();
exports.removeToken(matricola, function (error) {
if (error) {
callback(error);
return;
}
callback();
});
});

});
};

exports.removeToken = function (matricola, callback) {
var delToken = {Matricola: matricola,
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason of passing the matricola here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To use backend.writeStudentInfo() we have to pass matricola.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right

Token: "",
TokenDate: ""};
exports.writeStudentInfo(delToken, function (error) {
if (error) {
callback(error);
return;
}
console.log("backend: token removed for %s", delToken.Matricola);
callback();
});
};

exports.readStudentInfo = function (matricola, callback) {

console.info("backend: readStudentInfo");
Expand Down Expand Up @@ -169,6 +190,9 @@ function doWriteInfo(curInfo, callback) {
var knownKeys = {
"Nome": /^[A-Za-z\'\- ]+$/,
"Cognome": /^[A-Za-z\'\- ]+$/,
"Matricola": MATRICOLA,
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the matricola has always been part of the structure, and earlier it worked without adding the regexp for matricola in knownKeys, I'm not sure I get the rationale of adding the regexp to knownKeys now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do more tests, but probably when I removed it earlier not everything worked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Last time I tested the workflow of backend.hasValidKeys() and it worked without matricola in knownKeys. Unfortunately I haven't tested well backend.writeStudentInfo(newInfo). In general newInfo is an object with a matricola and other info to write. backend.writeStudentInfo() check the regexp before writing.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need to hurry. When you have time, can you please test all the code paths and report which is the code that fails if we remove (read: avoid to add in this pull request) the "Matricola" from `knownKeys'? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

writeStudentInfo() in /registry/backend.js from line 211 to line 218: if there isn't matricola in knownKeys it doesn't complete its workflow.

"Token": MAYBE_EMPTY_TOKEN,
"TokenDate": /^(|[0-9]{14})$/,
"Blog": URL,
"Twitter": /^(|@[A-Za-z0-9_]{1,15})$/,
"Wikipedia": /^(|(U|u)tente\:[^\{\}\[\]\#\|\<\>][^\{\}\[\]\#\|\<\>]+)$/,
Expand Down Expand Up @@ -212,12 +236,12 @@ exports.writeStudentInfo = function (newInfo, callback) {
for (index = 0; index < keys.length; ++index) {
key = keys[index];
if (knownKeys[key] === undefined) {
console.warn("backend: unknown key");
console.warn("backend: unknown key %s", key);
callback("backend: unknown key");
return;
}
if (newInfo[key].match(knownKeys[key]) === null) {
console.info("backend: regexp does not match");
console.info("backend: regexp does not match for %s", key);
callback("signup: regexp does not match");
return;
}
Expand Down
2 changes: 1 addition & 1 deletion registry/html/login_once.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ <h3>Scegli la tua password personale</h3>
$("#step2").html(data);
},
error: function (jqXHR, textStatus, errorThrown) {
$("#step2").html("<b>Si e' verificato un errore.</b>");
$("#step2").html("<b>"+jqXHR.responseText+"</b>");
}
});
return false;
Expand Down
20 changes: 20 additions & 0 deletions registry/login_once.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,26 @@ var handleRequest = function (request, response, matricola, token, hash) {
);
return;
}
var currentDate = new Date();
var tokenDate = new Date(obj.TokenDate.replace(
/^(\d{4})(\d\d)(\d\d)(\d\d)(\d\d)(\d\d)$/,'$4:$5:$6 $2/$3/$1'));
var differenceInMs = currentDate - tokenDate;
if (differenceInMs > 86400000) { //one day in ms
console.info("login_once: token expired");
backend.removeToken(matricola, function (error) {
if (error) {
callback(error);
return;
}
utils.writeHeadVerboseCORS(response, 500, {
"Content-Type": "text/plain"
});
response.end(
"La chiave inserita e' scaduta. Riparti da Sign Up o Reset password."
);
});
return;
}
console.info("login_once: right token --> saveUsers");
backend.saveUsers(matricola, hash, function (error) {
if (error) {
Expand Down
4 changes: 4 additions & 0 deletions registry/signup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ exports.handleMatricola = function (request, response) {

try {
studentInfo.Token = crypto.randomBytes(20).toString("hex");
studentInfo.TokenDate = new Date().toISOString()
.replace(/T/,'').replace(/\..+/, '').replace(/:/g,'')
.replace(/-/g,'');
} catch (error) {
utils.internalError(error, request, response);
return;
Expand Down Expand Up @@ -116,6 +119,7 @@ exports.handleMatricola = function (request, response) {
"Cognome": cognome,
"Matricola": message.Matricola,
"Token": "",
"TokenDate": "",
"Blog": "",
"Twitter": "",
"Wikipedia": "",
Expand Down