-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add enforcement for deviceAccess #4913
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.
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
Changes from 80 commits
8d635ff
3055b90
67fb91b
3ebb916
4f5c451
2dc6d1d
fa3e081
600a46e
7f578b3
176a312
9abf89c
6ce04ab
415e2f6
61fb82c
3cc4c67
cd81e02
e2b4e04
53b5970
5c00ed5
28848ad
ab635ee
93308f5
1cfd52d
d619807
9893f0f
986a251
2cae7c0
d7fd252
c7c01ba
24a28aa
7a1a4ec
4173d16
ca13952
0b6cd48
5f0110d
f930a34
9bff282
712ec9e
f192f65
006f53c
0341150
fa59897
dcff2cf
0222ce2
227beca
dc3ee49
9a5a08d
35ea5d4
f89b71c
4e1679b
fd80acd
2ab26a3
7de7ca6
5f1fe06
60b5ad8
5f44edd
4809354
8d8bdff
a1a2318
ac2ac12
dac2484
fd307e5
e5880f2
4240883
c18c61d
8f76851
473e107
b049ea2
c467a07
78664ba
0dbadf8
fd04c8d
fa07c42
5cae477
9336ea4
af229e7
6d064cf
c3c5b09
891ecbd
f281ee2
b0827c3
025886c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -83,8 +83,7 @@ function writeDigiId(id) { | |
var key = 'DigiTrust.v1.identity'; | ||
var date = new Date(); | ||
date.setTime(date.getTime() + 604800000); | ||
var exp = 'expires=' + date.toUTCString(); | ||
document.cookie = key + '=' + encId(id) + '; ' + exp + '; path=/;SameSite=none;'; | ||
utils.setCookie(key, encId(id), date.toUTCString(), 'none'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this may require more refactoring because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me look into this. The id is typically passed through and decoded via a server-side process. I'll see if there is a reasonable place to account for possible double encoding in that pipeline. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @goosemanjack - would like to get this ticket merged. Should we remove the call to encId()? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made a change here. Removed the call to encodeURI from encId. @goosemanjack - please review when you get a chance, but we need to merge this PR. |
||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -342,15 +342,15 @@ function getLanguage() { | |
|
||
function getLocalStorageSafely(key) { | ||
try { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hello @bretg! |
||
return localStorage.getItem(key); | ||
return utils.getDataFromLocalStorage(key); | ||
} catch (e) { | ||
return null; | ||
} | ||
} | ||
|
||
function setLocalStorageSafely(key, val) { | ||
try { | ||
return localStorage.setItem(key, val); | ||
return utils.setDataInLocalStorage(key, val); | ||
} catch (e) { | ||
return null; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
As
utils.getDataFromLocalStorage()
checks if localStorage is available and returns null if nothing is found, we can just use:Also, we call localStorage twice later in our bidder : lines 37 and 51. For this lines, we should use:
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@osazos - you can go ahead and make the change you've suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bretg, I created #4978.
Honestly, I don't know if there is a better way to submit my change. Please fell free to tell me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@osazos - I merged your changes from #4978 -- will close that PR.