Skip to content
This repository has been archived by the owner on May 7, 2021. It is now read-only.

feat(rpt-interceptor): add RPT refresh logic to auth interceptor & cleanup code #129

Merged
merged 1 commit into from
Sep 29, 2018

Conversation

rohitkrai03
Copy link
Contributor

Fixes fabric8-services/fabric8-auth#596

  • Adds logic to refresh RPT tokens by intercepting the response.
  • Adds logic to check for valid auth, wit or sso URLs where the interceptor needs to intercept.
  • Removes unnecessary use of isAuthenticationError since interceptor is going to handle that.
  • Adds related tests.
  • General code cleanup.

}
}

private isAuthenticationError(res: HttpErrorResponse): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't need isAuthenticationError then why was it moved in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do use isAuthenticationError but only inside the interceptor and that is why i moved it here. Earlier it was being used by other services as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not understanding why make the change. If it is external it keeps the code clean, it can be tested and it can be shared. It is more functional to keep it external.

What is the benefit of moving it inside?

Copy link
Contributor Author

@rohitkrai03 rohitkrai03 Sep 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will not be used anywhere else since interceptor does that already for every response. So, it made more sense to keep it tightly coupled as a bussiness logic to auth interceptor rather than keeping it external. Its just separation of concern for me.

Before moving to interceptors we had a shared http.service which had this logic but we moved it outside because we needed other services to explicitly check for the same errors. Since, we have interceptors now we do not need any other service to use this explicitly.

@joshuawilson joshuawilson merged commit 447c01e into fabric8-ui:master Sep 29, 2018
@fabric8cd
Copy link

🎉 This PR is included in version 2.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@rohitkrai03 rohitkrai03 deleted the rpt-interceptor branch October 3, 2018 10:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants