Skip to content

Commit 6bbb78a

Browse files
committed
fix: improve CSRF token handling and simplify error logging in API services
fix: enhance path validation for login/logout redirection refactor: streamline pager button text handling and remove unnecessary HTML escaping
1 parent f4471ea commit 6bbb78a

5 files changed

Lines changed: 57 additions & 105 deletions

File tree

src/IdentityManager2/Assets/Scripts/App/ttIdm.js

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@
1616
if (config.method && config.method !== 'GET') {
1717
try {
1818
const rvt = document.querySelector('input[name="__RequestVerificationToken"]');
19-
if (rvt) {
20-
const token = rvt.value;
21-
if (token) {
22-
if (!config.headers) {
23-
config.headers = {};
24-
}
25-
config.headers.RequestVerificationToken = token;
19+
if (rvt && rvt.value) {
20+
if (!config.headers) {
21+
config.headers = {};
2622
}
23+
config.headers.RequestVerificationToken = rvt.value;
24+
} else {
25+
// eslint-disable-next-line no-console
26+
console.warn("Anti-forgery token not present in DOM; state-changing request proceeds without CSRF protection.");
2727
}
2828
} catch (e) {
2929
// eslint-disable-next-line no-console
@@ -127,7 +127,7 @@
127127
if (resp.status === 403) {
128128
throw "You are not authorized to use this service.";
129129
} else {
130-
throw resp.data && (resp.data.exceptionMessage || resp.data.message) ||
130+
throw resp.data && resp.data.message ||
131131
"Failed to access IdentityManager API.";
132132
}
133133
});
@@ -138,7 +138,7 @@
138138
idmApi.$inject = ["$http", "$q", "PathBase"];
139139
app.factory("idmApi", idmApi);
140140

141-
function idmUsers($http, idmApi, $log) {
141+
function idmUsers($http, idmApi) {
142142
function nop() {
143143
}
144144

@@ -149,9 +149,6 @@
149149
function errorHandler(msg) {
150150
msg = msg || "Unexpected Error";
151151
return function (response) {
152-
if (response.data.exceptionMessage) {
153-
$log.error(response.data.exceptionMessage);
154-
}
155152
throw response.data.errors || response.data.message || msg;
156153
};
157154
}
@@ -213,10 +210,10 @@
213210

214211
return svc;
215212
}
216-
idmUsers.$inject = ["$http", "idmApi", "$log"];
213+
idmUsers.$inject = ["$http", "idmApi"];
217214
app.factory("idmUsers", idmUsers);
218215

219-
function idmRoles($http, idmApi, $log) {
216+
function idmRoles($http, idmApi) {
220217
function nop() {
221218
}
222219

@@ -227,9 +224,6 @@
227224
function errorHandler(msg) {
228225
msg = msg || "Unexpected Error";
229226
return function(response) {
230-
if (response.data.exceptionMessage) {
231-
$log.error(response.data.exceptionMessage);
232-
}
233227
throw response.data.errors || response.data.message || msg;
234228
};
235229
}
@@ -273,7 +267,7 @@
273267

274268
return svc;
275269
}
276-
idmRoles.$inject = ["$http", "idmApi", "$log"];
270+
idmRoles.$inject = ["$http", "idmApi"];
277271
app.factory("idmRoles", idmRoles);
278272
})(angular);
279273

src/IdentityManager2/Assets/Scripts/App/ttIdmApp.js

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,21 @@
5353

5454
load();
5555

56+
function isSafePath(path) {
57+
// Must start with a single '/' to prevent open redirect via protocol-relative URLs or javascript: schemes
58+
return typeof path === 'string' && path.length > 1 && path.charAt(0) === '/' && path.charAt(1) !== '/';
59+
}
60+
5661
$rootScope.login = function () {
5762
idmErrorService.clear();
58-
59-
$window.location = PathBase + (LoginPath || "/api/login");
63+
const path = isSafePath(LoginPath) ? LoginPath : "/api/login";
64+
$window.location = PathBase + path;
6065
};
6166

6267
$rootScope.logout = function() {
6368
idmErrorService.clear();
64-
65-
$window.location = PathBase + (LogoutPath || "/api/logout");
69+
const path = isSafePath(LogoutPath) ? LogoutPath : "/api/logout";
70+
$window.location = PathBase + path;
6671
};
6772
}
6873
LayoutCtrl.$inject = ["$rootScope", "PathBase", "idmApi", "$location", "$window", "idmErrorService", "ShowLoginButton",

src/IdentityManager2/Assets/Scripts/App/ttIdmUI.js

Lines changed: 7 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -173,33 +173,10 @@
173173
ttPagerSummary.$inject = ["PathBase"];
174174
app.directive("ttPagerSummary", ttPagerSummary);
175175

176-
function idmPager($sce) {
177-
function escapeHtml(value) {
178-
return String(value)
179-
.replace(/&/g, "&")
180-
.replace(/</g, "&lt;")
181-
.replace(/>/g, "&gt;")
182-
.replace(/"/g, "&quot;")
183-
.replace(/'/g, "&#39;");
184-
}
185-
186-
var allowedPagerHtml = {
187-
"<strong>&lt;&lt;</strong>": true,
188-
"<strong>&lt;</strong>": true,
189-
"<strong>&gt;</strong>": true,
190-
"<strong>&gt;&gt;</strong>": true
191-
};
192-
193-
function trustPagerText(text) {
194-
if (allowedPagerHtml[text]) {
195-
return $sce.trustAsHtml(text);
196-
}
197-
return $sce.trustAsHtml(escapeHtml(text));
198-
}
199-
176+
function idmPager() {
200177
function Pager(result, pageSize) {
201178
function PagerButton(text, page, enabled, current) {
202-
this.text = trustPagerText(text);
179+
this.text = String(text);
203180
this.page = page;
204181
this.enabled = enabled;
205182
this.current = current;
@@ -236,19 +213,19 @@
236213
var nextPage = this.currentPage + pageSkip;
237214
if (nextPage > this.totalPages) nextPage = this.totalPages;
238215

239-
this.buttons.push(new PagerButton("<strong>&lt;&lt;</strong>", 1, endButton > totalButtons));
240-
this.buttons.push(new PagerButton("<strong>&lt;</strong>", prevPage, endButton > totalButtons));
216+
this.buttons.push(new PagerButton("«", 1, endButton > totalButtons));
217+
this.buttons.push(new PagerButton("", prevPage, endButton > totalButtons));
241218

242219
for (var i = startButton; i <= endButton; i++) {
243220
this.buttons.push(new PagerButton(i, i, true, i === this.currentPage));
244221
}
245222

246-
this.buttons.push(new PagerButton("<strong>&gt;</strong>", nextPage, endButton < this.totalPages));
247-
this.buttons.push(new PagerButton("<strong>&gt;&gt;</strong>", this.totalPages, endButton < this.totalPages));
223+
this.buttons.push(new PagerButton("", nextPage, endButton < this.totalPages));
224+
this.buttons.push(new PagerButton("»", this.totalPages, endButton < this.totalPages));
248225
}
249226
return Pager;
250227
}
251-
idmPager.$inject = ["$sce"];
228+
idmPager.$inject = [];
252229
app.service("idmPager", idmPager);
253230

254231
function ttConfirmClick() {

src/IdentityManager2/Assets/Scripts/Bundle.js

Lines changed: 28 additions & 52 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<ul class="center-block pagination">
22
<li ng-repeat="button in pager.buttons"
33
ng-class="!button.enabled ? 'disabled' : (button.current ? 'active':'')">
4-
<a href="#/{{path}}/list/{{pager.filter}}/{{button.page}}" ng-bind-html="button.text"></a>
4+
<a href="#/{{path}}/list/{{pager.filter}}/{{button.page}}" ng-bind="button.text"></a>
55
</li>
66
</ul>

0 commit comments

Comments
 (0)