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.

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

Secure cookies #1167

Merged
merged 2 commits into from Dec 14, 2016
Merged

Secure cookies #1167

merged 2 commits into from Dec 14, 2016

Conversation

flaix
Copy link
Collaborator

@flaix flaix commented Dec 14, 2016

Gitblit already set the "secure" flag for cookies, i.e. send them only over secured transports, as well as 'httpOnly' flag for the Jetty session cookies already.
With this PR it will also do so for the user authentication cookies.

It will also set the 'secure' flag when HTTP is redirected to HTTPS by Gitblit.

fzs added 2 commits Dec 10, 2016
Mark the user authentication cookie to be only used for HTTP, making
it inaccessible for JavaScript engines.


If only HTTPS is used and no HTTP (i.e. also if HTTP is redirected to
HTTPS) then mark the user cookie to be sent only over secure connections.
So far for session cookies the secure property was only set when no
HTTP port was opened. This changes to also set it when HTTP is redirected
to the HTTPS port.
@flaix flaix added this to the 1.9.0 milestone Dec 14, 2016
@gitblit
Copy link
Owner

gitblit commented Dec 14, 2016

👍

@gitblit gitblit merged commit 4ece397 into gitblit:master Dec 14, 2016
@tomposmiko
Copy link

tomposmiko commented Dec 14, 2016

It fails to compile:

compile:
  [mx:keys] =========================================================
  [mx:keys] mx:Keys  (com.gitblit.Keys)
  [mx:keys] ---------------------------------------------------------
  [mx:keys]    input: /data/src/gitblit/src/main/distrib/data/defaults.properties
  [mx:keys]    output: /data/src/gitblit/src/main/java/com/gitblit/Keys.java
 [mx:javac] =========================================================
 [mx:javac] mx:Javac  (com.gitblit:gitblit:1.8.1-SNAPSHOT, compile)
 [mx:javac] ---------------------------------------------------------
 [mx:javac] Cleaning apt source directory src/main/gen
 [mx:javac] Cleaning output directory /data/src/gitblit/build/classes
 [mx:javac] Compiling 457 source files to /data/src/gitblit/build/classes
 [mx:javac] /data/src/gitblit/src/main/java/com/gitblit/models/UserModel.java:40: error: a type with the same simple name is already defined by the single-type-import of SecureRandom
 [mx:javac] import com.gitblit.utils.SecureRandom;
 [mx:javac] ^
 [mx:javac] /data/src/gitblit/src/main/java/com/gitblit/models/UserModel.java:669: error: cannot find symbol
 [mx:javac] 		return StringUtils.getSHA1(RANDOM.randomBytes(32));
 [mx:javac] 		                                 ^
 [mx:javac]   symbol:   method randomBytes(int)
 [mx:javac]   location: variable RANDOM of type SecureRandom
 [mx:javac] Note: Some input files use or override a deprecated API.
 [mx:javac] Note: Recompile with -Xlint:deprecation for details.
 [mx:javac] Note: Some input files use unchecked or unsafe operations.
 [mx:javac] Note: Recompile with -Xlint:unchecked for details.
 [mx:javac] 2 errors

BUILD FAILED
/data/src/gitblit/build.xml:110: Compile failed; see the compiler error output for details.

@gitblit
Copy link
Owner

gitblit commented Dec 15, 2016

Hey Tamas, the compile problem is not related to this PR. It does look like we'll need to review the class name and import collision on the SecureRandom class recently introduced.

@tomposmiko
Copy link

tomposmiko commented Dec 15, 2016

Do you want me to submit a separate issue?

@gitblit
Copy link
Owner

gitblit commented Dec 15, 2016

@tomposmiko Good idea.

@flaix
Copy link
Collaborator Author

flaix commented Dec 15, 2016

@tomposmiko, don't bother, it is a quick fix. Already pushed to master.

@tomposmiko
Copy link

tomposmiko commented Dec 18, 2016

Thanks, I didn't have time until now:)

@flaix flaix deleted the secureCookies branch Jun 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants