-
Notifications
You must be signed in to change notification settings - Fork 4
Feat/springsecurity #121
base: master
Are you sure you want to change the base?
Feat/springsecurity #121
Conversation
…of spring security
…ans in services, modified login authentication functionalty
…rated to ida project
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.
@deepakgarg08 @aliazmi68 I have left some comments, please compliance them
And remove all the necessary files or commented code if not needed!
There are one or two files which I believe not in use anymore (Backend file, I also mentioned them in comments)
And @maqboolayaz I would you to review backend code and comment here if everything is fine!
hi,
I have no info about this file.
ida-ws/src/main/java/upb/ida/smtp/EmailForSignup.java
<dice-group/IDA#121 (comment)>:
you can delete this file
ida-ws/src/main/java/upb/ida/repository/UserRepository.java
<dice-group/IDA#121 (comment)>:
don't delete this file, its being used
ida-ws/src/main/java/upb/ida/rest/LoginController.java
<dice-group/IDA#121 (comment)>:
…On Sat, Jun 22, 2019 at 1:01 PM maq ***@***.***> wrote:
***@***.**** requested changes on this pull request.
@deepakgarg08 <https://github.com/deepakgarg08> @aliazmi68
<https://github.com/aliazmi68> I have left some comments, please
compliance them
And remove all the necessary files or commented code if not needed!
There are one or two files which I believe not in use anymore (Backend
file, I also mentioned them in comments)
And @maqboolayaz <https://github.com/maqboolayaz> I would you to review
backend code and comment here if everything is fine!
------------------------------
In ida-ws/src/main/java/upb/ida/rest/LoginController.java
<dice-group/IDA#121 (comment)>:
> @@ -0,0 +1,70 @@
+package upb.ida.rest;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.springframework.beans.factory.annotation.Autowired;
Are we really using this file???
------------------------------
In ida-chatbot/src/app/components/user/user.component.html
<dice-group/IDA#121 (comment)>:
> + <mat-menu #actionsmenu="matMenu">
+ <button mat-icon-button color="primary" (click)="startEdit(row.username, row.firstname, row.lastname)">
+ <mat-icon aria-label="Edit">edit</mat-icon>Edit
+ </button><br>
+ <button mat-icon-button color="warn" (click)="deleteItem(row.username)">
+ <mat-icon aria-label="Delete">delete</mat-icon>Delete
+ </button>
+ </mat-menu>
+ </td>
+ </ng-container>
+
+ <tr mat-header-row *matHeaderRowDef="displayedColumns"></tr>
+ <tr mat-row *matRowDef="let row; columns: displayedColumns;"></tr>
+ </table>
+ </div>
+ <div *ngIf="this.dataSource.data.length === 0">
you dont need === 0 part here
------------------------------
In ida-chatbot/src/app/components/user/user.component.ts
<dice-group/IDA#121 (comment)>:
> + dataSource = new MatTableDataSource();
+ showSpinner = false;
+
+ constructor(private restservice: RestService, private userservice: UserService, private router: Router, public dialog: MatDialog , public snackBar: MatSnackBar) {}
+
+ ngOnInit() {
+ this.isAdmin();
+ }
+
+ startEdit(username: string, firstname: string, lastname: string) {
+
+ const dialogRef = this.dialog.open(UpdateDialogComponent, {
+ data: {username: username, firstname: firstname, lastname: lastname}
+ });
+
+ dialogRef.afterClosed().subscribe(result => {
error handling is missing in the whole file
------------------------------
In ida-chatbot/src/app/components/user/user.component.ts
<dice-group/IDA#121 (comment)>:
> + }
+ if(counter == 5){
+ counter = 0;
+ if(user.userrole == "USER"){
+ users.push(user);
+ }
+ user = {
+ 'firstname':'',
+ 'lastname':'',
+ 'username':'',
+ 'userrole':'',
+ 'password':'',
+ };
+ }
+ }
+ if (returnResp.status === false) {
if (! returnResp.status)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<dice-group/IDA#121>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFSNP2KJYXA5YABCEF2IF5DP3YA75ANCNFSM4GRIKNUA>
.
|
ida-ws/src/main/java/upb/ida/security/CustomAuthenticationProvider.java
Outdated
Show resolved
Hide resolved
Hi, all extra files and extra commented code removed
…On Mon, Jul 1, 2019 at 11:43 AM Ayaz Maqbool ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In ida-ws/src/main/resources/application.properties
<dice-group/IDA#121 (comment)>:
> @@ -2,3 +2,25 @@ logging.level.root=INFO
spring.devtools.restart.enabled=true
server.servlet.contextPath=/ida-ws
+
+# ===============================
+# = DATA SOURCE
+# ===============================
+# Set here configurations for the database connection
+#spring.data.uri=http://localhost:3030
Remove the neo4j db configurations from here.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<dice-group/IDA#121>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFSNP2MKFMNQ4TP7E7CJG3TP5HGUBANCNFSM4GRIKNUA>
.
|
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.
Please resolve the merge conflict asap so we can include this feature in our demo. Ignore the missing tests for now.
Now you can merge |
cc @Cortys |
@maqboolkhan This feature is already merged into our demo branch. We could also merge it into master but that would decrease our test coverage since no tests have been written yet for the new user management code. @nikit91 Should we merge this in now and add the tests in a separate PR next semester or delay the merge? |
…ode quality issues.
Implementation of Spring Security:
Implementation of Angular UI: