Skip to content

Commit 2e62fab

Browse files
smarterclaytonsmarterclayton
committed
Merge pull request #8958 from liggitt/session-storage-1.2
CSRF updates - 1.2.0
2 parents e0b4ef6 + 56ebf57 commit 2e62fab

File tree

10 files changed

+476
-51
lines changed

10 files changed

+476
-51
lines changed

‎assets/app/scripts/controllers/util/oauth.js

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,22 @@
88
* Controller of the openshiftConsole
99
*/
1010
angular.module('openshiftConsole')
11-
.controller('OAuthController', function ($location, $q, RedirectLoginService, DataService, AuthService, Logger) {
11+
.controller('OAuthController', function ($scope, $location, $q, RedirectLoginService, DataService, AuthService, Logger) {
1212
var authLogger = Logger.get("auth");
1313

14+
// Initialize to a no-op function.
15+
// Needed to let the view confirm a login when the state is unverified.
16+
$scope.completeLogin = function(){};
17+
$scope.cancelLogin = function() {
18+
$location.replace();
19+
$location.url("./");
20+
};
21+
1422
RedirectLoginService.finish()
1523
.then(function(data) {
1624
var token = data.token;
1725
var then = data.then;
26+
var verified = data.verified;
1827
var ttl = data.ttl;
1928

2029
// Try to fetch the user
@@ -25,21 +34,41 @@ angular.module('openshiftConsole')
2534
.then(function(user) {
2635
// Set the new user and token in the auth service
2736
authLogger.log("OAuthController, got user", user);
28-
AuthService.setUser(user, token, ttl);
2937

30-
// Redirect to original destination (or default to '/')
31-
var destination = then || './';
32-
if (URI(destination).is('absolute')) {
33-
authLogger.log("OAuthController, invalid absolute redirect", destination);
34-
destination = './';
38+
$scope.completeLogin = function() {
39+
// Persist the user
40+
AuthService.setUser(user, token, ttl);
41+
42+
// Redirect to original destination (or default to './')
43+
var destination = then || './';
44+
if (URI(destination).is('absolute')) {
45+
authLogger.log("OAuthController, invalid absolute redirect", destination);
46+
destination = './';
47+
}
48+
authLogger.log("OAuthController, redirecting", destination);
49+
$location.replace();
50+
$location.url(destination);
51+
};
52+
53+
if (verified) {
54+
// Automatically complete
55+
$scope.completeLogin();
56+
} else {
57+
// Require the UI to prompt
58+
$scope.confirmUser = user;
59+
60+
// Additionally, give the UI info about the user being overridden
61+
var currentUser = AuthService.UserStore().getUser();
62+
if (currentUser && currentUser.metadata.name !== user.metadata.name) {
63+
$scope.overriddenUser = currentUser;
64+
}
3565
}
36-
authLogger.log("OAuthController, redirecting", destination);
37-
$location.url(destination);
3866
})
3967
.catch(function(rejection) {
4068
// Handle an API error response fetching the user
4169
var redirect = URI('error').query({error: 'user_fetch_failed'}).toString();
4270
authLogger.error("OAuthController, error fetching user", rejection, "redirecting", redirect);
71+
$location.replace();
4372
$location.url(redirect);
4473
});
4574

@@ -51,6 +80,7 @@ angular.module('openshiftConsole')
5180
error_uri: rejection.error_uri || ""
5281
}).toString();
5382
authLogger.error("OAuthController, error", rejection, "redirecting", redirect);
83+
$location.replace();
5484
$location.url(redirect);
5585
});
5686

‎assets/app/scripts/services/login.js

Lines changed: 75 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,70 @@ angular.module('openshiftConsole')
2929
this.$get = function($location, $q, Logger) {
3030
var authLogger = Logger.get("auth");
3131

32+
var getRandomInts = function(length) {
33+
var randomValues;
34+
35+
if (window.crypto && window.Uint32Array) {
36+
try {
37+
var r = new Uint32Array(length);
38+
window.crypto.getRandomValues(r);
39+
randomValues = [];
40+
for (var j=0; j < length; j++) {
41+
randomValues.push(r[j]);
42+
}
43+
} catch(e) {
44+
authLogger.debug("RedirectLoginService.getRandomInts: ", e);
45+
randomValues = null;
46+
}
47+
}
48+
49+
if (!randomValues) {
50+
randomValues = [];
51+
for (var i=0; i < length; i++) {
52+
randomValues.push(Math.floor(Math.random() * 4294967296));
53+
}
54+
}
55+
56+
return randomValues;
57+
};
58+
59+
var nonceKey = "RedirectLoginService.nonce";
60+
var makeState = function(then) {
61+
var nonce = String(new Date().getTime()) + "-" + getRandomInts(8).join("");
62+
try {
63+
window.localStorage[nonceKey] = nonce;
64+
} catch(e) {
65+
authLogger.log("RedirectLoginService.makeState, localStorage error: ", e);
66+
}
67+
return JSON.stringify({then: then, nonce:nonce});
68+
};
69+
var parseState = function(state) {
70+
var retval = {
71+
then: null,
72+
verified: false
73+
};
74+
75+
var nonce = "";
76+
try {
77+
nonce = window.localStorage[nonceKey];
78+
window.localStorage.removeItem(nonceKey);
79+
} catch(e) {
80+
authLogger.log("RedirectLoginService.parseState, localStorage error: ", e);
81+
}
82+
83+
try {
84+
var data = state ? JSON.parse(state) : {};
85+
if (data && data.nonce && nonce && data.nonce === nonce) {
86+
retval.verified = true;
87+
retval.then = data.then;
88+
}
89+
} catch(e) {
90+
authLogger.error("RedirectLoginService.parseState, state error: ", e);
91+
}
92+
authLogger.error("RedirectLoginService.parseState", retval);
93+
return retval;
94+
};
95+
3296
return {
3397
// Returns a promise that resolves with {user:{...}, token:'...', ttl:X}, or rejects with {error:'...'[,error_description:'...',error_uri:'...']}
3498
login: function() {
@@ -49,7 +113,7 @@ angular.module('openshiftConsole')
49113
uri.query({
50114
client_id: _oauth_client_id,
51115
response_type: 'token',
52-
state: returnUri.toString(),
116+
state: makeState(returnUri.toString()),
53117
redirect_uri: _oauth_redirect_uri
54118
});
55119
authLogger.log("RedirectLoginService.login(), redirecting", uri.toString());
@@ -59,7 +123,7 @@ angular.module('openshiftConsole')
59123
},
60124

61125
// Parses oauth callback parameters from window.location
62-
// Returns a promise that resolves with {token:'...',then:'...'}, or rejects with {error:'...'[,error_description:'...',error_uri:'...']}
126+
// Returns a promise that resolves with {token:'...',then:'...',verified:true|false}, or rejects with {error:'...'[,error_description:'...',error_uri:'...']}
63127
// If no token and no error is present, resolves with {}
64128
// Example error codes: https://tools.ietf.org/html/rfc6749#section-5.2
65129
finish: function() {
@@ -71,12 +135,12 @@ angular.module('openshiftConsole')
71135
var fragmentParams = new URI("?" + u.fragment()).query(true);
72136
authLogger.log("RedirectLoginService.finish()", queryParams, fragmentParams);
73137

74-
// Error codes can come in query params or fragment params
75-
// Handle an error response from the OAuth server
138+
// Error codes can come in query params or fragment params
139+
// Handle an error response from the OAuth server
76140
var error = queryParams.error || fragmentParams.error;
77-
if (error) {
78-
var error_description = queryParams.error_description || fragmentParams.error_description;
79-
var error_uri = queryParams.error_uri || fragmentParams.error_uri;
141+
if (error) {
142+
var error_description = queryParams.error_description || fragmentParams.error_description;
143+
var error_uri = queryParams.error_uri || fragmentParams.error_uri;
80144
authLogger.log("RedirectLoginService.finish(), error", error, error_description, error_uri);
81145
return $q.reject({
82146
error: error,
@@ -85,13 +149,16 @@ angular.module('openshiftConsole')
85149
});
86150
}
87151

152+
var stateData = parseState(fragmentParams.state);
153+
88154
// Handle an access_token response
89155
if (fragmentParams.access_token && (fragmentParams.token_type || "").toLowerCase() === "bearer") {
90156
var deferred = $q.defer();
91157
deferred.resolve({
92158
token: fragmentParams.access_token,
93159
ttl: fragmentParams.expires_in,
94-
then: fragmentParams.state
160+
then: stateData.state,
161+
verified: stateData.verified
95162
});
96163
return deferred.promise;
97164
}

‎assets/app/views/util/oauth.html

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,26 @@
22
<div class="wrap no-sidebar">
33
<div class="middle surface-shaded">
44
<div class="container surface-shaded">
5-
<div>
5+
<div ng-if="!confirmUser">
66
<h1 style="margin-top: 10px;">Logging in&hellip;</h1>
7-
<div>
8-
Please wait while you are logged in...
9-
</div>
7+
<p>Please wait while you are logged in&hellip;</p>
108
</div>
9+
10+
<div ng-if="confirmUser && !overriddenUser">
11+
<h1 style="margin-top: 10px;">Confirm Login</h1>
12+
<p>You are being logged in as <code>{{confirmUser.metadata.name}}</code>.</p>
13+
<button class="btn btn-lg btn-primary" type="button" ng-click="completeLogin();">Continue</button>
14+
<button class="btn btn-lg btn-default" type="button" ng-click="cancelLogin();">Cancel</button>
15+
</div>
16+
17+
<div ng-if="confirmUser && overriddenUser">
18+
<h1 style="margin-top: 10px;">Confirm User Change</h1>
19+
<p>You are about to change users from <code>{{overriddenUser.metadata.name}}</code> to <code>{{confirmUser.metadata.name}}</code>.</p>
20+
<p>If this is unexpected, click Cancel. This could be an attempt to trick you into acting as another user.</p>
21+
<button class="btn btn-lg btn-danger" type="button" ng-click="completeLogin();">Switch Users</button>
22+
<button class="btn btn-lg btn-primary" type="button" ng-click="cancelLogin();">Cancel</button>
23+
</div>
24+
1125
</div>
1226
</div>
1327
</div>

0 commit comments

Comments
 (0)