Skip to content

Commit cc49cba

Browse files
brockallenbrockallen
committed
rework how state is modeled
1 parent 94023b5 commit cc49cba

9 files changed

Lines changed: 181 additions & 152 deletions

‎src/OidcClientService.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@ export default class OidcClientService {
3030
return this._metadataService.getAuthorizationEndpoint().then(url => {
3131
Log.info("Received authorization endpoint", url);
3232

33-
return new SigninRequest(url,
33+
return new SigninRequest({url,
3434
client_id,
3535
redirect_uri,
3636
response_type,
3737
scope,
38-
state);
38+
state});
3939
}, err => {
4040
Log.error("Failed to create signin request", err);
4141
throw new Error("Failed to create signin request");
@@ -56,7 +56,7 @@ export default class OidcClientService {
5656
return this._metadataService.getEndSessionEndpoint().then(url => {
5757
Log.info("Received end session endpoint", url);
5858

59-
return new SignoutRequest(url, {
59+
return new SignoutRequest({url,
6060
id_token_hint,
6161
post_logout_redirect_uri,
6262
state});

‎src/SigninRequest.js‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import UrlUtility from './UrlUtility';
33
import State from './State';
44

55
export default class SigninRequest {
6-
constructor(url, client_id, redirect_uri, response_type, scope, state, StateCtor = State) {
6+
constructor({url, client_id, redirect_uri, response_type, scope, state}) {
77

88
if (!url){
99
Log.error("No url passed to SigninRequest");
@@ -31,23 +31,40 @@ export default class SigninRequest {
3131
this._redirect_uri = redirect_uri;
3232
this._response_type = response_type;
3333
this._scope = scope;
34-
this._state = new StateCtor(state);
34+
this._state = new State({nonce:this.isOidc, data:state});
35+
}
36+
37+
get state(){
38+
return this._state;
3539
}
3640

3741
get signinUrl(){
3842
if (!this._signinUrl){
3943
this._signinUrl = this._url;
40-
this._signinUrl = UrlUtility.addQueryParam(this._signinUrl, "state", this._state.toUriString());
4144
this._signinUrl = UrlUtility.addQueryParam(this._signinUrl, "client_id", this._client_id);
4245
this._signinUrl = UrlUtility.addQueryParam(this._signinUrl, "redirect_uri", this._redirect_uri);
4346
this._signinUrl = UrlUtility.addQueryParam(this._signinUrl, "response_type", this._response_type);
4447
this._signinUrl = UrlUtility.addQueryParam(this._signinUrl, "scope", this._scope);
48+
this._signinUrl = UrlUtility.addQueryParam(this._signinUrl, "state", this._state.id);
49+
if (this.isOidc){
50+
this._signinUrl = UrlUtility.addQueryParam(this._signinUrl, "nonce", this._state.nonce);
51+
}
4552
}
4653

4754
return this._signinUrl;
4855
}
49-
50-
get state(){
51-
return this._state;
56+
57+
get isOidc() {
58+
var result = this._response_type.split(/\s+/g).filter(function(item) {
59+
return item === "id_token";
60+
});
61+
return !!(result[0]);
62+
}
63+
64+
get isOAuth() {
65+
var result = this._response_type.split(/\s+/g).filter(function(item) {
66+
return item === "token";
67+
});
68+
return !!(result[0]);
5269
}
5370
}

‎src/SignoutRequest.js‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import UrlUtility from './UrlUtility';
33
import State from './State';
44

55
export default class SignoutRequest {
6-
constructor(url, {id_token_hint, post_logout_redirect_uri, state} = {}, StateCtor = State) {
6+
constructor({url, id_token_hint, post_logout_redirect_uri, state}) {
77
if (!url) {
88
Log.error("No url passed to SignoutRequest");
99
throw new Error("url");
@@ -13,13 +13,17 @@ export default class SignoutRequest {
1313
this._id_token_hint = id_token_hint;
1414
this._post_logout_redirect_uri = post_logout_redirect_uri;
1515

16-
this._state = new StateCtor(state);
16+
this._state = new State({data:state});
17+
}
18+
19+
get state(){
20+
return this._state;
1721
}
1822

1923
get signoutUrl() {
2024
if (!this._signoutUrl) {
2125
this._signoutUrl = this._url;
22-
this._signoutUrl = UrlUtility.addQueryParam(this._signoutUrl, "state", this._state.toUriString());
26+
this._signoutUrl = UrlUtility.addQueryParam(this._signoutUrl, "state", this._state.id);
2327

2428
if (this._id_token_hint) {
2529
this._signoutUrl = UrlUtility.addQueryParam(this._signoutUrl, "id_token_hint", this._id_token_hint);
@@ -32,8 +36,4 @@ export default class SignoutRequest {
3236

3337
return this._signoutUrl;
3438
}
35-
36-
get state(){
37-
return this._state;
38-
}
3939
}

‎src/State.js‎

Lines changed: 22 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,37 @@
11
import Log from './Log';
22
import random from './random';
3-
import Crypto from './Crypto';
43

54
export default class State {
6-
constructor(state, crypto = Crypto){
7-
8-
this._data = {nonce:random()};
9-
this._data.state = state;
10-
11-
this._crypto = crypto;
5+
constructor({id, nonce, data}={}) {
6+
this._id = id || random();
7+
if (nonce === true){
8+
this._nonce = random();
9+
}
10+
else if (nonce){
11+
this._nonce = nonce;
12+
}
13+
this._data = data;
1214
}
1315

1416
get id() {
15-
return this._data.nonce;
17+
return this._id;
1618
}
17-
18-
toUriString(){
19-
return encodeURIComponent(JSON.stringify(this._data));
19+
get nonce() {
20+
return this._nonce;
2021
}
21-
22-
toClientStorageString(){
22+
get data() {
23+
return this._data;
24+
}
25+
26+
toStorageString() {
2327
return JSON.stringify({
24-
hash: this._crypto.hash(this.toUriString()),
25-
nonce: this._data.nonce
28+
id:this.id,
29+
nonce:this.nonce,
30+
data:this.data
2631
});
2732
}
2833

29-
static verify(clientStorageString, stateString, crypto = Crypto){
30-
Log.info("State.verify");
31-
32-
var clientStorage = JSON.parse(clientStorageString);
33-
34-
if (clientStorage.hash === crypto.hash(stateString)){
35-
Log.info("hash comparison successful");
36-
37-
var json = decodeURIComponent(stateString);
38-
var data = JSON.parse(json);
39-
40-
if (data.nonce === clientStorage.nonce) {
41-
Log.info("nonce comparison successful");
42-
43-
return data.state || true;
44-
}
45-
else{
46-
Log.warn("nonce comparison failed");
47-
}
48-
}
49-
else {
50-
Log.warn("hash comparison failed");
51-
}
52-
53-
return false;
34+
static fromStorageString(storageString){
35+
return new State(JSON.parse(storageString));
5436
}
5537
}

‎test/OidcClientService.spec.js‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,13 @@ describe("OidcClientService", function() {
6969
});
7070

7171
p.then(request => {
72+
request.state.data.should.equal('foo');
73+
7274
var url = request.signinUrl;
7375
url.should.contain("http://sts/authorize");
74-
url.should.contain("foo");
75-
url.should.contain("bar");
76-
url.should.contain("baz");
77-
url.should.contain("quux");
76+
url.should.contain("response_type=bar");
77+
url.should.contain("scope=baz");
78+
url.should.contain("redirect_uri=quux");
7879
done();
7980
});
8081
});
@@ -120,11 +121,11 @@ describe("OidcClientService", function() {
120121
});
121122

122123
p.then(request => {
124+
request.state.data.should.equal('foo');
123125
var url = request.signoutUrl;
124126
url.should.contain("http://sts/signout");
125-
url.should.contain("foo");
126-
url.should.contain("bar");
127-
url.should.contain("baz");
127+
url.should.contain("post_logout_redirect_uri=bar");
128+
url.should.contain("id_token_hint=baz");
128129
done();
129130
});
130131
});

‎test/SigninRequest.spec.js‎

Lines changed: 65 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,32 @@
11
import Log from '../src/Log';
22
import SigninRequest from '../src/SigninRequest';
33

4-
import StubState from './StubState';
5-
64
import chai from 'chai';
75
chai.should();
86
let assert = chai.assert;
97

108
describe("SigninRequest", function() {
11-
12-
let stubState = new StubState("state");
13-
let StubStateCtor = () => stubState;
14-
9+
1510
let subject;
16-
17-
beforeEach(function(){
18-
subject = new SigninRequest("http://sts/signin", "client", "http://app", "id_token", "openid", undefined, StubStateCtor);
11+
let settings;
12+
13+
beforeEach(function() {
14+
settings = {url: "http://sts/signin",
15+
client_id: "client",
16+
redirect_uri: "http://app",
17+
response_type: "id_token",
18+
scope: "openid",
19+
state: {data: "test"}
20+
};
21+
subject = new SigninRequest(settings);
1922
});
2023

2124
describe("constructor", function() {
2225

2326
it("should require a url param", function() {
2427
try {
25-
new SigninRequest(undefined, "client", "http://app", "id_token", "openid");
28+
delete settings.url;
29+
new SigninRequest(settings);
2630
}
2731
catch (e) {
2832
e.message.should.contain('url');
@@ -32,7 +36,8 @@ describe("SigninRequest", function() {
3236
});
3337
it("should require a client_id param", function() {
3438
try {
35-
new SigninRequest("http://sts/signin", undefined, "http://app", "id_token", "openid");
39+
delete settings.client_id;
40+
new SigninRequest(settings);
3641
}
3742
catch (e) {
3843
e.message.should.contain('client_id');
@@ -42,7 +47,8 @@ describe("SigninRequest", function() {
4247
});
4348
it("should require a redirect_uri param", function() {
4449
try {
45-
new SigninRequest("http://sts/signin", "client", undefined, "id_token", "openid");
50+
delete settings.redirect_uri;
51+
new SigninRequest(settings);
4652
}
4753
catch (e) {
4854
e.message.should.contain('redirect_uri');
@@ -52,7 +58,8 @@ describe("SigninRequest", function() {
5258
});
5359
it("should require a response_type param", function() {
5460
try {
55-
new SigninRequest("http://sts/signin", "client", "http://app", undefined, "openid");
61+
delete settings.response_type;
62+
new SigninRequest(settings);
5663
}
5764
catch (e) {
5865
e.message.should.contain('response_type');
@@ -62,7 +69,8 @@ describe("SigninRequest", function() {
6269
});
6370
it("should require a scope param", function() {
6471
try {
65-
new SigninRequest("http://sts/signin", "client", "http://app", "id_token", undefined);
72+
delete settings.scope;
73+
new SigninRequest(settings);
6674
}
6775
catch (e) {
6876
e.message.should.contain('scope');
@@ -72,7 +80,7 @@ describe("SigninRequest", function() {
7280
});
7381

7482
});
75-
83+
7684
describe("signinUrl", function() {
7785

7886
it("should include url", function() {
@@ -91,9 +99,49 @@ describe("SigninRequest", function() {
9199
subject.signinUrl.should.contain("scope=openid");
92100
});
93101
it("should include state", function() {
94-
subject.signinUrl.should.contain("state=state");
102+
subject.signinUrl.should.contain("state=" + subject.state.id);
103+
});
104+
105+
});
106+
107+
describe("isOidc", function() {
108+
it("should indicate if response_type is oidc", function() {
109+
settings.response_type = "id_token";
110+
subject = new SigninRequest(settings);
111+
subject.isOidc.should.be.true;
112+
113+
settings.response_type = "id_token token";
114+
subject = new SigninRequest(settings);
115+
subject.isOidc.should.be.true;
116+
117+
settings.response_type = "token id_token";
118+
subject = new SigninRequest(settings);
119+
subject.isOidc.should.be.true;
120+
121+
settings.response_type = "token";
122+
subject = new SigninRequest(settings);
123+
subject.isOidc.should.be.false;
124+
});
125+
});
126+
127+
describe("isOAuth", function() {
128+
it("should indicate if response_type is oauth", function() {
129+
settings.response_type = "token";
130+
subject = new SigninRequest(settings);
131+
subject.isOAuth.should.be.true;
132+
133+
settings.response_type = "id_token token";
134+
subject = new SigninRequest(settings);
135+
subject.isOAuth.should.be.true;
136+
137+
settings.response_type = "token id_token";
138+
subject = new SigninRequest(settings);
139+
subject.isOAuth.should.be.true;
140+
141+
settings.response_type = "id_token";
142+
subject = new SigninRequest(settings);
143+
subject.isOAuth.should.be.false;
95144
});
96-
97145
});
98146

99147
});

0 commit comments

Comments
 (0)