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

Creates Exception for Invalid URLs passed to Redirect Filter #95

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

BrendanErquiaga
Copy link
Contributor

SecureRedirectFilter validates URLs before attempting to filter them, failures throw an InvalidURLException.

This will be used to reduce noise in the #custom-lob-alerts room.

This has not been practically tested, only unit tested. I am currently not 100% sure how to deploy this.

SecureRedirectFilter validates URLs before attempting to filter them, failures throw a InvalidURLException
Added tests for this
@dvallelonga
Copy link

@BrendanErquiaga I think we have a problem that could throw a wrench in your efforts to deploy this fix: Cadmium seems to be in a state right now where backend/war deploys are broken/not doable. 🙁

I've pasted a few emails from Chris Haley below regarding some issues he ran into earlier this year and what his findings and conclusion were. Specifically, I'm referencing Email 3/3, Gotcha # 1. I've included the other emails for background in case you're interested.

Email 3/3 (Mar 12, 2018)

Hey Everyone,

After spending close to two weeks attempting to fix Cadmium, I have come to the conclusion that too much time has been consumed on this effort without success. I have put together a confluence page on how we can work around the issue.

Bottom line is that GitHub did a security upgrade that doesn't play nice with the version of Java that Cadmium was written in.
We will need to use the steps provided in the confluence page for content updates moving forward.

Gotchas

  1. We will not be able to do any backend/war updates on cadmium (this should be low risk).
  2. Updating in Genentech environment will require a ticket and someone from their team will need to execute.

Thanks,
Chris Haley

Email 2/3 (Mar 6, 2018)

Hey everyone,

I'm running into a little bit of a wall here. :(

I finally got java 1.7 installed on cd.meltdev.com after doing that I was able to send the deploy war command to the server. Which lead to a new exception. One that really has me scratching my head.
Which I have attached to this email.

My goal tonight was to get rituxan-ra-hcp-car-updates.cd.meltdev.com deployed tonight but this blocked that.

I will try to dig into this a little more in the morning.

Possible workaround

  1. We could manually create the static content for each deploy/update. Which is the out/ directory that is created when docpad does it's thing.
  2. zip up the out directory
  3. ssh the out directory to the server
  4. stop jboss
  5. replace the content in the sites cadmium directory
  6. start jboss

No war updates would be able to be done if we when with this 'workaround'

Again I'll continue to work on this tomorrow. My day is looking pretty light so I should be able to focus on it all day.

Thank
Chris

Email 1/3 (Feb 26, 2018)

Hey everyone,

So I found two major issues today.

  1. Update the call to https://api.github.com/authorizations to include fingerprints. This was preventing cadmium-admin and the CLI from authenticating.
  2. Updated the org.eclipse.jgit dependency. This was preventing us from being able to clone and update git repositories when populating and updating cadmium content.
    1. Looks like this version of jgit does not play nice with java 1.7. So I'm unsure at this point if these new wars will run in jboss.

Next Steps

  1. Look into why cadmium-admin is still broken.
    1. This may take a little bit of time for me to remember how to get this thing up and running locally. I'm currently fighting with auth when running locally. I know I did this years ago so it might just take me some time to reopen that part of my brain.
  2. Test new cadmium wars in meltdev
  3. Build a plan for getting these changes into meltdev/meltqa/gene-qa/production

Warren/Robin,
Where should I log my time? Should I keep moving forward with this effort? I'm not really sure we have an option at this point.

Thanks,
Chris Haley

@BrendanErquiaga
Copy link
Contributor Author

We will not be deploying this fix due to the current state of cadmium that Dave pointed out. But the PR will remain open in the event that we decide to update cadmium in the future.

We disabled the alerts in loggly to avoid spam in the slack channel.

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.

2 participants